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

Attribute limits not supported in Metrics #1830

Open
jtmalinowski opened this issue Jul 27, 2021 · 3 comments
Open

Attribute limits not supported in Metrics #1830

jtmalinowski opened this issue Jul 27, 2021 · 3 comments
Labels
area:data-model For issues related to data model area:sdk Related to the SDK release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:metrics Related to the specification/metrics directory

Comments

@jtmalinowski
Copy link
Contributor

jtmalinowski commented Jul 27, 2021

What are you trying to achieve?

As #1130 is nearing completion we're aiming to support Attribute limits in all models, which have Attributes. However, Metrics cause additional difficulties, because truncation and/or deletion of their attributes could affect their identity, which requires further discussion.

Additional context.

Some notes from a comment on open-telemetry/opentelemetry-specification#1130:

And some more ideas from another comment in open-telemetry/opentelemetry-specification#1130:

  • Limiting attribute length can lead to duplicate (identity) metric points being exported, especially for verbose labels where the tailing-end of the label contains the important part. E.g. `"processor"; "com.java.really.long.package.name.FooBar". In this case, we should be encouraging users to use the shortest, readable metric attributes, but is otherwise considered an error state.
  • Limiting the # of attributes could lead to duplicate metric time series being emitted from the SDK. Here we should inform users that this is considered a configuration error, and we expect them to provide a high enough value for metric streams or provide View aggregation to limit high attribute-count metrics. We can leave the error reporting + mechanism for this TBD, but we need to call out this is a very real problem in the current (stable) data model.
@jtmalinowski jtmalinowski added the spec:metrics Related to the specification/metrics directory label Jul 27, 2021
@jtmalinowski
Copy link
Contributor Author

I'm happy to own this one and drive the discussion, unless someone working on Metrics would like to own it.

@manolama
Copy link

HI! Starting to look into supporting OTel but I'm worried about this idea of truncating data. The limits make absolute sense and being configurable is great. But to avoid user confusion, avoid the duplicated identity issue, and the problems of how to truncate UTF-8 from #504, wouldn't it be better to drop the errant data and notify the user? E.g. have the APIs return a response code, 0 for good, non-zero for a data error. Then use the built-in SDK telemetry to track the errors. I think truncation will lead to a world of hurt.

@reyang reyang added area:api Cross language API specification issue area:sdk Related to the SDK labels Sep 8, 2021
@reyang reyang self-assigned this Sep 8, 2021
@reyang reyang added this to the Metrics API/SDK Feature Freeze milestone Sep 8, 2021
@reyang
Copy link
Member

reyang commented Sep 30, 2021

Two updates:

  • I've sent a PR to give some guidance/suggestion on how to handle memory limits Add metrics spec supplementary guidelines #1966.
  • I think the Metrics SDK spec will not define the exact behavior in the initial Stable release (so the SDK authors can have flexibility), as we called out here.

@reyang reyang added area:data-model For issues related to data model and removed area:api Cross language API specification issue labels Oct 1, 2021
@reyang reyang added the release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs label Oct 1, 2021
@reyang reyang removed this from the Metrics API/SDK Feature Freeze milestone Oct 1, 2021
@reyang reyang unassigned reyang and jmacd Oct 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:data-model For issues related to data model area:sdk Related to the SDK release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:metrics Related to the specification/metrics directory
Projects
None yet
Development

No branches or pull requests

4 participants