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

[obsreport] Replace Prometheus namespace with metrics prefix #5939

Closed

Conversation

dmitryax
Copy link
Member

"otelcol" part of the exposed metrics that is set as Prometheus namespace is replaced the same explicit metrics prefix. This change brings names consistency between metrics that can be reported with enabled or disabled "telemetry.useOtelForInternalMetrics" feature gate.

Resolves #5882

@dmitryax dmitryax requested a review from a team as a code owner August 21, 2022 16:41
@dmitryax dmitryax force-pushed the replace-prom-namespace branch 7 times, most recently from 8c86f0b to 288dece Compare August 21, 2022 17:37
"otelcol" part of the exposed metrics that is set as Prometheus namespace is replaced the same explicit metrics prefix. This change brings names consistency between metrics that can be reported with enabled or disabled "telemetry.useOtelForInternalMetrics" feature gate.
@codecov
Copy link

codecov bot commented Aug 21, 2022

Codecov Report

Merging #5939 (0bb640b) into main (f0d69d6) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #5939      +/-   ##
==========================================
- Coverage   91.93%   91.93%   -0.01%     
==========================================
  Files         200      200              
  Lines       12406    12404       -2     
==========================================
- Hits        11406    11404       -2     
  Misses        789      789              
  Partials      211      211              
Impacted Files Coverage Δ
exporter/exporterhelper/obsreport.go 100.00% <100.00%> (ø)
obsreport/obsreport_exporter.go 100.00% <100.00%> (ø)
obsreport/obsreport_receiver.go 100.00% <100.00%> (ø)
obsreport/obsreport_scraper.go 100.00% <100.00%> (ø)
obsreport/obsreporttest/obsreporttest.go 95.62% <100.00%> (ø)
service/internal/telemetry/process_telemetry.go 71.55% <100.00%> (ø)
service/telemetry.go 90.25% <100.00%> (-0.13%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2022

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 Sep 5, 2022
@bogdandrutu bogdandrutu removed the Stale label Sep 9, 2022
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.

Just one comment, otherwise this looks good. Hurray for consistency :)

@@ -4,6 +4,9 @@

### 🛑 Breaking changes 🛑

- "otelcol" part of the exposed metrics that is set as Prometheus namespace is replaced the same explicit metrics prefix (#5939)
Copy link
Contributor

Choose a reason for hiding this comment

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

To be clear to most users this will not be a breaking change as the prometheus metric exposed will be the same.

Suggested change
- "otelcol" part of the exposed metrics that is set as Prometheus namespace is replaced the same explicit metrics prefix (#5939)
- Update `otelcol_` part of the exposed Prometheus metrics to be set via a metric prefix instead of a namespace (#5939)

@github-actions
Copy link
Contributor

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

@paivagustavo
Copy link
Member

paivagustavo commented Oct 3, 2022

This can also be accomplished by wrapping the prometheus.Registry before registering the otel-go prom exporter's Collector, which would not break any additional metric:
https://github.com/open-telemetry/opentelemetry-collector/pull/6222/files#diff-52da5566fa270d6a253f7a301b66f4a1dbda7c2c42f3781f8357efe7f08dff10R255

edit:
see #6223

@bogdandrutu
Copy link
Member

#6223 closes this

@bogdandrutu bogdandrutu closed this Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Collector metrics names reported with OpenCensus and OpenTelemetry are inconsistent
4 participants