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 general aggregations into Metric interface #3670

Closed
yantang-msft opened this issue Jan 13, 2018 · 8 comments
Closed

Add general aggregations into Metric interface #3670

yantang-msft opened this issue Jan 13, 2018 · 8 comments
Labels
area/metrics feature request Requests for new plugin and for new features to existing plugins waiting for response waiting for response from contributor

Comments

@yantang-msft
Copy link

I'm trying to write a output plugin to Azure Application Insights, which supports aggregated value natively in its datapoint.
The problem is for an aggregated metric, I won't know which aggregator emitted it. Thus the conversion logic can be funky. It also doesn't scale if a new aggregator is added.
So I'd like to have the Metric interface expose those aggregations (count, min, max, quantiles...) along with the IsAggregate() method.

@danielnelson
Copy link
Contributor

Why is it important to know which aggregator emitted the metric?

@danielnelson danielnelson added feature request Requests for new plugin and for new features to existing plugins area/metrics labels Jan 13, 2018
@yantang-msft
Copy link
Author

@danielnelson The App Insights has its own aggregated telemetry defined. https://github.com/Microsoft/ApplicationInsights-Go/blob/master/appinsights/telemetry.go#L194
I'll have to convert the aggregated metric from telegraf to App Insights' format. Otherwise, the backend of App Insights won't be able to understand the metric correctly.

Currently, to convert the aggregated metric from telegraf to App Insights, I'll need to parse each field based on the format of the aggregator plugins. But as an output plugin, I shouldn't need to know what aggregator is used, given count, min, max... are general aggregations.

@danielnelson
Copy link
Contributor

What if you just treat, for example, any field named "*_min" as a AggregateMetricTelemetry with a Min field without checking if it is IsAggregate()? I can imagine there could be some false positives but I think that would be rare, and it would allow the output to work without knowledge of the source.

@yantang-msft
Copy link
Author

@danielnelson Yes, I can do that, but as you said, it's kind of unreliable.
Also, it requires all aggregator plugins' field follows the same pattern. And in this case, I'd rather have the aggregator plugins conform to the pattern explicitly by having these field strongly typed.

@danielnelson
Copy link
Contributor

Ultimately someone would need to guess in many cases as well, such as when the data originates upstream from a queuing system via a plugin like kafka_consumer.

We currently have something similar in the Type() function which is used by the prometheus output. It has a few shortcomings, primarily that it applies to the entire Metric. I am working on some changes to the Metric interface for 1.6, and I hope to rework this aspect, so I'll see if I can provide what you need as part of this.

@danielnelson danielnelson added this to the 1.6.0 milestone Jan 16, 2018
@danielnelson danielnelson self-assigned this Feb 1, 2018
@danielnelson danielnelson modified the milestones: 1.6.0, 1.7.0 Mar 22, 2018
@danielnelson
Copy link
Contributor

Adding this support on a per field basis while not taking a performance hit way is proving to be a bit more work and won't make it in for 1.6.

@srebhan
Copy link
Member

srebhan commented Aug 15, 2023

@yantang-msft are you still interested in this matter?

@srebhan srebhan added the waiting for response waiting for response from contributor label Aug 15, 2023
@telegraf-tiger
Copy link
Contributor

Hello! I am closing this issue due to inactivity. I hope you were able to resolve your problem, if not please try posting this question in our Community Slack or Community Forums or provide additional details in this issue and reqeust that it be re-opened. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics feature request Requests for new plugin and for new features to existing plugins waiting for response waiting for response from contributor
Projects
None yet
Development

No branches or pull requests

4 participants