-
Notifications
You must be signed in to change notification settings - Fork 207
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
Support Application Signals .NET runtime metrics exporting #1471
base: main
Are you sure you want to change the base?
Conversation
} | ||
|
||
func NewAggregationMutator() AggregationMutator { | ||
return newAggregationMutatorWithConfig(map[string]aggregationType{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently this aggregation mutator is specific to DotNet. Do we plan on having this interface be generic to support other languages?
I would have a config mapping specific to each language
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also how does this differ from the current runtime metrics we support? Curious why DotNet requires significant code changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The aggregation is specific to .NET because .NET SDK doesn't provide a mechanism to change the aggregation in the View
model due to its immaturity. We don't have the same problem in the other languages because their SDKs have provided relatively complete support for the View
model which allows us to tackle the problem at SDK side.
See open-telemetry/opentelemetry-python-contrib#2861 (comment) for how we resolved the same issue in Python SDK.
See open-telemetry/opentelemetry-dotnet#2618 for the current feature development status of View
in .NET.
The aggregation mutator is served as an interim solution before .NET supports the aggregation configuration in View
.
generation: gen0 | ||
include: process.runtime.dotnet.gc.collections.count | ||
match_type: "" | ||
new_name: DotNetGCGen0Count |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The config is getting quite large...is there a way of having a wildcard for each language?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can give it a try, but by peeking at the code, the chance is low.
We take label from metrics and make it part of the new metric name using experimental_match_labels
to filter and new_name
to define an exact name. On the other hand, the wildcard matching and value assignment in metricstransform
only takes the original metric name and the new metric name into consideration,
if newName := transform.MetricIncludeFilter.expand(transform.NewName, metric.Name()); newName != "" {
metric.SetName(newName)
} else {
metric.SetName(transform.NewName)
}
We cannot configure the new_name to follow our naming strategy (with label values included).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested, same conclusion.
Description of changes
This PR adds metrics transformation and conversion for Application Signals .NET runtime metrics.
Related PR:
aws/amazon-cloudwatch-agent-operator#279
aws-observability/helm-charts#148
License
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Tests
Describe what tests you have done.
Requirements
Before commit the code, please do the following steps.
make fmt
andmake fmt-sh
make lint