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

[processor/groupbytrace] Update metric units #34167

Closed
wants to merge 2 commits into from

Conversation

honeychaudharyc
Copy link
Contributor

@honeychaudharyc honeychaudharyc commented Jul 19, 2024

Fix for processor/groupbytraceprocessor component

  • processor/groupbytraceprocessor

Link to tracking Issue: #34143

This updates units for groupbytraceprocessor internal telemetry.
@honeychaudharyc honeychaudharyc requested a review from a team July 19, 2024 06:54
@github-actions github-actions bot added the processor/groupbytrace Group By Trace processor label Jul 19, 2024
@@ -6,66 +6,66 @@

The following telemetry is emitted by this component.

### otelcol_processor_groupbytrace_conf_num_traces
### processor_groupbytrace_conf_num_traces
Copy link
Member

Choose a reason for hiding this comment

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

do you know what happened here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ran go generate ./... and it was updated automatically. I ran this to make sure documentation is up to date with metadata.yaml

Copy link
Member

@jpkrohling jpkrohling Jul 19, 2024

Choose a reason for hiding this comment

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

@codeboten, do you know if this is concerning?

@crobert-1
Copy link
Member

I believe this should have a changelog

@jpkrohling
Copy link
Member

I agree: there should be a changelog for the unit change, and there should not be a change in the metric name. I know @codeboten worked on adding the "otel_" prefix automatically and I think it would reflect in the documentation at the individual components, so this prefix change on this PR looks strange.

Copy link
Contributor

github-actions bot commented Aug 3, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Aug 3, 2024
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

I think the unit updates look good. As @jpkrohling mentioned, a changelog entry for this would be great. I'm not seeing the prefix change mentioned, has that been addressed?

@github-actions github-actions bot removed the Stale label Aug 13, 2024
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Aug 27, 2024
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
processor/groupbytrace Group By Trace processor Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants