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

prometheus: add WithNamespace option to prefix metrics #3970

Merged

Conversation

paivagustavo
Copy link
Member

Adds a WithNamespace option to the prometheus exporter.

Currently, the lack of such option blocks the otel collector: open-telemetry/opentelemetry-collector#7454

To finalize the collector migration to otel-go and start enabling it by default, both instrumentations must be compatible.

Collector's OpenCensus instrumentation prefixes metrics with the otelcol_ prefix.
To match that behavior in the otel-go instrumentation, we tried to wrap a prometheus.Registry with the otelcol_ prefix.

But since Otel Go resource attributes are exported as part of the target_info metric, which is also prefixed and becomes otelcol_target_info, for example:

# HELP otelcol_otel_scope_info Instrumentation Scope metadata
# TYPE otelcol_otel_scope_info gauge
otelcol_otel_scope_info{otel_scope_name="testmeter",otel_scope_version="v0.1.0"} 1
# HELP otelcol_target_info Target metadata
# TYPE otelcol_target_info gauge
otelcol_target_info{service_name="prometheus_test",telemetry_sdk_language="go",telemetry_sdk_name="opentelemetry",telemetry_sdk_version="latest"} 1

This make it impossible for the OpenTelemetry Collector's Prometheus Receiver to properly parse the target_info and otel_scope_info into the appropriated resource attributes and scope attributes. For that reason, the Otel Go instrumentation is not compatible with the existing OpenCensus instrumentation.

Adding such a common option in the prometheus instrumentation would unblock the Collector's migration to Otel Go and also other external users such as: #3405 (comment)

For reference, this option is also available on collector's prometheus exporter

closes #3437

@codecov
Copy link

codecov bot commented Apr 4, 2023

Codecov Report

Merging #3970 (5f89a61) into main (02fa1e2) will decrease coverage by 0.1%.
The diff coverage is 100.0%.

❗ Current head 5f89a61 differs from pull request most recent head 0976db2. Consider uploading reports for the commit 0976db2 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #3970     +/-   ##
=======================================
- Coverage   82.0%   82.0%   -0.1%     
=======================================
  Files        171     171             
  Lines      12955   12965     +10     
=======================================
+ Hits       10635   10641      +6     
- Misses      2100    2104      +4     
  Partials     220     220             
Impacted Files Coverage Δ
exporters/prometheus/config.go 100.0% <100.0%> (ø)
exporters/prometheus/exporter.go 83.9% <100.0%> (+0.2%) ⬆️

... and 3 files with indirect coverage changes

CHANGELOG.md Outdated Show resolved Hide resolved
paivagustavo and others added 2 commits April 5, 2023 13:04
Co-authored-by: Robert Pająk <pellared@hotmail.com>
@MrAlias MrAlias added this to the v1.15.0 milestone Apr 6, 2023
@MadVikingGod MadVikingGod merged commit 1c05d9c into open-telemetry:main Apr 6, 2023
19 checks passed
@paivagustavo paivagustavo deleted the gustavo.prom_with_namespace branch April 7, 2023 12:40
@MrAlias MrAlias mentioned this pull request Apr 27, 2023
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.

Add a WithNamespace option to the Prometheus Exporter
7 participants