-
Notifications
You must be signed in to change notification settings - Fork 16
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
Feature Suggestion: Add subscriptions as tags #9
Comments
Hey @timbutler, you bring up an excellent point! I'm sure many others would like the ability to add check subscriptions as tags. I do want to point out that not every event being handled is guaranteed to contain a check, for example events created by Sensu's statsd server. At the very least, we would want to verify the event contains a check before assuming its existence. I would also suggest changing the tags to a more traditional key/value set, and perhaps even allowing the user to pass a
There's a lot of room for improvement here! We initially only covered the base use case, but I'd love to see the functionality you're describing, including some other customization such as a |
Hi @Nikkiki, thanks for the response! The issue with the basic K/V set (unless I'm overlooking something?) is that Influx don't allow the same key for tags per metric and the example above would also only pass the last entry. I also thought about simply sending the subscription slice as a simple delimited string (eg It's possible that I've overthought the answer however, appreciate the work on the handler so far and will publish some checks (which return Influx metrics) over the next week as well to suit. |
@timbutler Oh okay, I see what you're saying. What about the following example? I'm not an expert at Influx Query Language, but it seems like you could query based on substrings of the key (ex. does it contain 'sensu_subscription'). I'm just trying to ensure consistency and context (ex.
|
Hi @Nikkiki, I had it initially like this, but then ran into the issue of the order of the subscriptions. For example, if I had Here's an example of the query I'm using to filter: This allows me to select all checks from within Note: I have to filter out the entity by name as well as I'm using the agents more as proxies for remote checks, otherwise the inclusion would be fine. I'm sure there's probably a neater method than my quick fix, ideally I'd like the way to dynamically create filters (eg, so I could select via Thanks for your input so far, it's been greatly appreciated. |
Hi there, I'm still pretty unexperienced here so please forgive me if I make mistakes. If I understand correctly than the current behaviour is that tagging is limited to the influxdb tag "sensu_entity_name" (which maps the the sensu attribute "event.Entity.Name" and you exchange here for adding a tag for the the sensu attribute "event.Check.Subscription". Wouldn't it be possible (and good approach) that an adminstrator himself creates and maintains the mapping as he needs. |
Correct, in that scenario both would be hardcoded. @marcseguin I totally agree, it's probably best that we keep the mapping as generic as possible so administrators can customize their tags. I think it's worth bringing up the sensu-go-metric-tag-enrichment mutator that @calebhailey hacked together for this use case. Using a mutator to map and inject metric tags for check fields, entity fields, and labels is definitely the more Sensu way. |
There's also this idea: sensu/sensu-go#2160 |
Just started logging some of the metrics to Influx, but found one minor part missing. When using subscriptions to group items (eg part of "web" and also part of "production"), adding additional tags to the Influx data means this can be used for filtering. This way, data can be shown for all "web" systems and/or also all "production".
Since Influx doesn't easily allow multiple values for the same tag, I've taken a slightly different approach with a fork of the code here: timbutler@f66796d
However, there's probably a neater way of handling this but still allowing easy grouping. Is there a better approach and has anyone else raised this as a request?
The text was updated successfully, but these errors were encountered: