-
Notifications
You must be signed in to change notification settings - Fork 213
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
Set default value of flattenAttributes to true in Otel metrics source #4190
Set default value of flattenAttributes to true in Otel metrics source #4190
Conversation
Signed-off-by: Krishna Kondaka <[email protected]>
Signed-off-by: Krishna Kondaka <[email protected]>
@@ -85,7 +81,7 @@ public Collection<Record<? extends Metric>> doExecute(Collection<Record<?>> reco | |||
for (Record<?> rec : records) { | |||
if ((rec.getData() instanceof Event)) { | |||
Record<? extends Metric> newRecord = (Record<? extends Metric>)rec; | |||
if (otelMetricsRawProcessorConfig.getFlattenAttributesFlag() || | |||
if (!otelMetricsRawProcessorConfig.getFlattenAttributesFlag() || |
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.
Can you explain why this is being negated?
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.
It's true by default. Only when it is set to false in the config, we call JacksonMetric.setFlattenAttributes(false)
Signed-off-by: Krishna Kondaka <[email protected]>
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.
Aren't these changes going to break existing pipelines? This makes the data fundamentally different.
@dlvenable. No It doesn't change existing setup |
Signed-off-by: Krishna Kondaka <[email protected]>
Signed-off-by: Krishna Kondaka <[email protected]>
Signed-off-by: Krishna Kondaka <[email protected]> Co-authored-by: Krishna Kondaka <[email protected]>
Description
Set default value of flattenAttributes to true in Otel metrics source.
Modify the OTEL metrics source to create events/records with flatten attributes set to true.
If OTEL processor config changes the flatten attributes to flase, then change it false in the processor.
OTEL metrics always have been implemented with flattening the attributes(ie fields inside "attributes" field are moved to the root). We have added option to not flatten some time back. But that's not used by anyone (that I know of). When the previous PR which moved the creation of OTEL metrics records to OTEL source (instead of otel processor) I picked the default to false and it is changed to true in the processor. But that was unnecessary also hit a bug due to valid characters in a key. This new approach will avoid it.
Issues Resolved
Resolves #[Issue number to be closed when this PR is merged]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.