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

Increase metric name maximum length from 63 to 255 characters #3648

Merged
merged 5 commits into from
Aug 10, 2023

Conversation

trask
Copy link
Member

@trask trask commented Aug 8, 2023

Addresses part of #3422

Changes

Increases metric name maximum length from 63 to 255 characters.

Rationale

This limitation has been problematic in the Java and .NET communities (at least) where longer metric names are not uncommon:

As @aabmass mentions in #3422 (comment)

Looks like this [the 63 character limitation] was introduced in #1557 by @reyang

but I didn't see any rationale in that PR for why that number was chosen.

Is this a breaking change?

I don't think so, since it's a loosening (as opposed to strengthening) of the requirement, but you could also argue the alternative, since it's a "strengthening" of the requirement on telemetry consumers.

Alternatively, we could introduce a configurable setting for the metric name maximum length, but this would essentially add an (unbounded) strengthening of the requirement on telemetry consumers, so I'm not sure it's preferable.

@trask trask requested review from a team August 8, 2023 23:40
CHANGELOG.md Outdated Show resolved Hide resolved
@reyang
Copy link
Member

reyang commented Aug 8, 2023

Is this a breaking change?

I don't think so, since it's a loosening (as opposed to strengthening) of the requirement, but you could also argue the alternative, since it's a "strengthening" of the requirement on telemetry consumers.

Alternatively, we could introduce a configurable setting for the metric name maximum length, but this would essentially add an (unbounded) strengthening of the requirement on telemetry consumers, so I'm not sure it's preferable.

+1

@reyang
Copy link
Member

reyang commented Aug 9, 2023

I think this change is non-controversial (very small change, not breaking, doesn't require prototyping, sufficient demand), and can be included in 1.24.0 release? #3644 - FYI @carlosalberto

@joaopgrassi
Copy link
Member

I did some looking on what most o11y vendors tell on their public pages about metric name/key size just as an FYI

I think it should be fine since we are increasing the limit. If a back-end that accepted less than 255 truncated the metric name to fit their limit, would still continuing truncating as usual.

@reyang
Copy link
Member

reyang commented Aug 10, 2023

@trask would you update the changelog by moving the entry from 1.24.0 to unreleased?

@reyang reyang merged commit 0a12882 into open-telemetry:main Aug 10, 2023
6 checks passed
@trask trask deleted the increase-metric-name-size-limit branch August 10, 2023 17:59
@nayaniabhishek
Copy link

@trask
Copy link
Member Author

trask commented Aug 13, 2023

hi @nayaniabhishek, here's the Java PR: open-telemetry/opentelemetry-java#5697 (I'm also looking forward to this change)

but please note that it won't be merged until there has been a specification release that includes this spec change (see #2728 (comment))

mstoykov added a commit to grafana/k6 that referenced this pull request Sep 18, 2023
This is inline with the current restriction.

The previous 63 character one came from Otel, but with
open-telemetry/opentelemetry-specification#3648
This has been bumped to 255.

Updates #3065
mstoykov added a commit to grafana/k6 that referenced this pull request Sep 18, 2023
This is inline with the current restriction.

The previous 63 character one came from Otel, but with
open-telemetry/opentelemetry-specification#3648
This has been bumped to 255.

Updates #3065
mstoykov added a commit to grafana/k6 that referenced this pull request Sep 20, 2023
This is inline with the current restriction.

The previous 63 character one came from Otel, but with
open-telemetry/opentelemetry-specification#3648
This has been bumped to 255.

Updates #3065
mstoykov added a commit to grafana/k6 that referenced this pull request Sep 20, 2023
This is inline with the current restriction.

The previous 63 character one came from Otel, but with
open-telemetry/opentelemetry-specification#3648
This has been bumped to 255.

Updates #3065
carlosalberto pushed a commit that referenced this pull request Sep 22, 2023
## Changes

Updated the following entry in the spec compliance matrix:

* Section Metrics, item "Instrument names conform to the specified
syntax."
* Language C++

The lastest spec changes (#3648, #3422) are implemented by the following
PR:

* open-telemetry/opentelemetry-cpp#2281
* open-telemetry/opentelemetry-cpp#2303

so C++ can be marked compliant with the specification.
@kalkibhagavanmeda
Copy link

Hi team
Any expectations like when will be releasing latest version of opentelemetry-api with increased metrics name length limitation

@trask
Copy link
Member Author

trask commented Nov 7, 2023

hi @kalkibhagavanmeda, I'd suggest asking this in the language repository for the SDK you are using, e.g. this is implemented in the latest released Java SDK version (open-telemetry/opentelemetry-java#5697)

carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
…n-telemetry#3703)

## Changes

Updated the following entry in the spec compliance matrix:

* Section Metrics, item "Instrument names conform to the specified
syntax."
* Language C++

The lastest spec changes (open-telemetry#3648, open-telemetry#3422) are implemented by the following
PR:

* open-telemetry/opentelemetry-cpp#2281
* open-telemetry/opentelemetry-cpp#2303

so C++ can be marked compliant with the specification.
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.

10 participants