Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add queue name to state reporting #7537

Closed
wants to merge 1 commit into from
Closed

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented Jul 9, 2018

The queue name is reported as part of the state event. An event looks as following:

{
  "beat": {
    "name": "ruflin"
  },
  "host": {
    "architecture": "x86_64",
    "name": "ruflin",
    "os": {
      "build": "17E202",
      "family": "darwin",
      "platform": "darwin",
      "version": "10.13.4"
    }
  },
  "module": {
    "count": 3,
    "names": [
      "system"
    ]
  },
  "output": {
    "name": "elasticsearch"
  },
  "queue": {
    "name": "mem"
  },
  "service": {
    "id": "0e1b1913-73db-4512-b17a-8209fa45eaa4",
    "name": "metricbeat",
    "version": "7.0.0-alpha1"
  }
}

The queue name is reported as part of the state event. An event looks as following:

```
{
  "beat": {
    "name": "ruflin"
  },
  "host": {
    "architecture": "x86_64",
    "name": "ruflin",
    "os": {
      "build": "17E202",
      "family": "darwin",
      "platform": "darwin",
      "version": "10.13.4"
    }
  },
  "module": {
    "count": 3,
    "names": [
      "system"
    ]
  },
  "output": {
    "name": "elasticsearch"
  },
  "queue": {
    "name": "mem"
  },
  "service": {
    "id": "0e1b1913-73db-4512-b17a-8209fa45eaa4",
    "name": "metricbeat",
    "version": "7.0.0-alpha1"
  }
}
```
@@ -157,6 +157,10 @@ func createQueueBuilder(config common.ConfigNamespace) (func(queue.Eventer) (que
queueConfig = common.NewConfig()
}

stateRegistry := monitoring.GetNamespace("state").GetRegistry()
queueRegistry := stateRegistry.NewRegistry("queue")
monitoring.NewString(queueRegistry, "name").Set(queueType)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pipeline should not use any globals at all, so one can safely instantiate multiple pipelines via Load. The Load function even gets the metrics registry passed as parameter (optional, can be nil).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The registry passed is not the same into which this data has to go.

If there can be multiple queues in the future we would need a variable inside this package to keep track of it like we do for modules. Would that be ok for you?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand it's a different regsitry. In this case we should pass a 'telemetry' registry or some kind of callback to the loader. But no globals. It's one of the few packages with no globals in mind from the very beginning.

Me might have other places where we need to pass a telemetry and a metrics registry to a feature/module/plugin. Maybe we want to pass a struct or interface of type:

type monitoring interface {
  Telemetry() *monitoring.Registry
  Metrics() *monitoring.Registry
  Logs() logp.Logger
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ruflin I see some globals have been sneaked in... see #8091 for cleanups + introduction of queue type telemetry

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for opening #8091 It seems this make this PR obsolete, right?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

@ruflin
Copy link
Contributor Author

ruflin commented Aug 27, 2018

Closing in favor of #8091

@ruflin ruflin closed this Aug 27, 2018
@ruflin ruflin deleted the queue-stats branch August 27, 2018 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants