Skip to content

sdk_exporters/prometheus: Clarify Content Negotiation #4543

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Jun 24, 2025
Merged

sdk_exporters/prometheus: Clarify Content Negotiation #4543

merged 7 commits into from
Jun 24, 2025

Conversation

@ArthurSens
Copy link
Member

@ArthurSens ArthurSens commented Jun 4, 2025 •
edited
Loading

Changes

Addressing feedback received during the last Spec SIG meeting, I'm splitting my previous PR(#4533) into two to ensure we keep the scope of the original one for the configuration changes.

If I understand correctly, almost all SDKs (if not all) are already compliant since they are wrappers around Prometheus SDKs, and Prometheus SDKs already do content negotiation properly. Then the PoCs are already done :)

@ArthurSens ArthurSens requested review from a team June 4, 2025 21:50
@ArthurSens
Copy link
Member Author

ArthurSens commented Jun 4, 2025

Once this gets in, #4494 will complement the content negotiation section just to clarify how translation strategies interact with content negotiation

dashpole
dashpole reviewed Jun 5, 2025
View reviewed changes
@ArthurSens
Copy link
Member Author

ArthurSens commented Jun 5, 2025

It's not 100% copy-and-paste; some things got moved around. But yes, the content is basically the same.

If we just add a reference to Prometheus website, then we're at risk of specification changing without us noticing if the content of Prometheus's website changes. Is that an ok risk to take?

@dashpole
Copy link
Contributor

dashpole commented Jun 5, 2025

I think that is OK for a few reasons:

  1. OTel Prom exporters mostly use the Prometheus client libraries, which should stay up-to-date with those docs.
  2. It is fairly safe to assume that those docs will not change in backwards-incompatible ways, since that would mean prometheus clients would also have to make breaking changes.

@ArthurSens ArthurSens requested a review from dashpole June 5, 2025 18:03
pellared
pellared reviewed Jun 5, 2025
View reviewed changes
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Just please add a changelog entry 😉

@pellared pellared added spec:metrics Related to the specification/metrics directory changelog.opentelemetry.io area:sdk Related to the SDK labels Jun 5, 2025
@ArthurSens
Copy link
Member Author

ArthurSens commented Jun 6, 2025

Overall LGTM. Just please add a changelog entry 😉

I'm looking at all the CHANGELOG sections and I'm bit confused about which part should I add an entry. Is it Metrics? Compatibility? SDK Configuration?

@ArthurSens ArthurSens force-pushed the prom-content-negotiation branch from a586521 to a762009 Compare June 6, 2025 13:00
@pellared
Copy link
Member

pellared commented Jun 6, 2025

Overall LGTM. Just please add a changelog entry 😉

I'm looking at all the CHANGELOG sections and I'm bit confused about which part should I add an entry. Is it Metrics? Compatibility? SDK Configuration?

I propose metrics.

ArthurSens added 4 commits June 6, 2025 17:45
@ArthurSens
sdk_exporters/prometheus: Clarify Content Negotiation
094a951 
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
@ArthurSens
Just add a link to Prometheus docs
95d3c91 
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
@ArthurSens
Adjust line width to ~80
9064cac 
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
@ArthurSens
Changelog entry
bc423e4 
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
@ArthurSens ArthurSens force-pushed the prom-content-negotiation branch from fc12e53 to bc423e4 Compare June 6, 2025 20:46
@reyang reyang enabled auto-merge June 19, 2025 22:37
@reyang reyang disabled auto-merge June 19, 2025 22:37
@reyang
Copy link
Member

reyang commented Jun 19, 2025 •
edited
Loading

@ArthurSens Would you move the changelog entry from the released section (v1.46.0 (2025-06-12)) to the unreleased section? Thanks!

@dashpole
Copy link
Contributor

dashpole commented Jun 23, 2025

@ArthurSens is OOO for a while. I moved the changelog entry to unreleased

@carlosalberto carlosalberto added this pull request to the merge queue Jun 24, 2025
Merged via the queue into open-telemetry:main with commit 867445f Jun 24, 2025
6 checks passed
@carlosalberto carlosalberto mentioned this pull request Jul 10, 2025
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jul 18, 2025
@carlosalberto @arminru @trask
Release 1.47.0 (#4584)
4988a46 
July 2025 release.

### Context

- Add Supplementary Guidelines for environment variables as context
carrier
  specification.

([#4548](#4548))

### Traces

- Define sampling threshold field in OpenTelemetry TraceState; define
the behavior
of TraceIdRatioBased sampler in terms of W3C Trace Context Level 2
randomness.

([#4166](#4166))
- Define CompositeSampler implementation and built-in ComposableSampler
interfaces.

([#4466](#4466))
- Define how SDK implements `Tracer.Enabled`.

([#4537](#4537))

### Logs

- Stabilize `Event Name` parameter of `Logger.Enabled`.

([#4534](#4534))
- Stabilize SDK and No-Op `Logger.Enabled`.

([#4536](#4536))
- `SeverityNumber=0` MAY be used to represent an unspecified value.

([#4535](#4535))

### Baggage

- Add Supplementary Guidelines for environment variables as context
carrier
  specification.

([#4548](#4548))

### Compatibility

- Clarify expectations about Prometheus content negotiation for metric
names.

([#4543](#4543))

---------

Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dashpole dashpole

@pellared pellared

@reyang reyang

Assignees

No one assigned

Labels

area:sdk Related to the SDK changelog.opentelemetry.io spec:metrics Related to the specification/metrics directory

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ArthurSens @dashpole @pellared @reyang @carlosalberto