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

View attribute filtering is not compliant with the specification #4156

Closed
MrAlias opened this issue May 31, 2023 · 5 comments · Fixed by #4288 or open-telemetry/opentelemetry-specification#3680
Assignees
Labels
area:metrics Part of OpenTelemetry Metrics pkg:SDK Related to an SDK package
Milestone

Comments

@MrAlias
Copy link
Contributor

MrAlias commented May 31, 2023

The specification states the stream configuration of a view must have an allow-list of attribute keys that it will use to filter out attributes:

  • A list of attribute keys (optional). If provided, the attributes that are not in the list will be ignored. If not provided, all the attribute keys will be used by default (TODO: once the Hint API is available, the default behavior should respect the Hint if it is available).

However, the opentelemetry-go implementation uses an attribute.Filter.

The non-compliance of the implementation needs to be addressed

Proposal

Update the specification to allow something like the attribute.Filter to be used for this functionality.

Having a list of attribute keys that are allowed is in the stream configuration limits the usefulness of attribute filtering. It only allows an allow-list type filtering for static pre-determined attribute keys. Having something allowed like the attribute.Filter allows for deny-lists, filtering on values, fuzzy filtering algorithms, probabilistic filtering ... It seems like users would benefit by keeping the attribute.Filter here instead of replacing it with a []attribute.Key field.

Alternatives

Replace the attribute.Filter type with []attribute.Key in the Stream.AttributeFilter field. Use these keys to filter attributes.

@MrAlias MrAlias added pkg:SDK Related to an SDK package area:metrics Part of OpenTelemetry Metrics labels May 31, 2023
@MrAlias MrAlias moved this from Todo to Blocked in Go: Metric SDK (GA) May 31, 2023
@MrAlias
Copy link
Contributor Author

MrAlias commented Jun 1, 2023

@MrAlias
Copy link
Contributor Author

MrAlias commented Jun 8, 2023

Specification PR: open-telemetry/opentelemetry-specification#3550

@MrAlias
Copy link
Contributor Author

MrAlias commented Jul 6, 2023

Adding this to the specification does not appear viable. Moving to update our implemenetation.

@MrAlias MrAlias self-assigned this Jul 6, 2023
@MrAlias MrAlias moved this from Blocked to In Progress in Go: Metric SDK (GA) Jul 6, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in Go: Metric SDK (GA) Jul 8, 2023
@MrAlias MrAlias added this to the v1.17.0/v0.40.0 milestone Aug 3, 2023
@MrAlias
Copy link
Contributor Author

MrAlias commented Aug 11, 2023

The resolution applied makes the fix for #3765, and likely other future fixes, impossible.

Alternative discussed in yesterdays SIG meeting:

This seems like it would be in line with the specification in spirit, a user would be able to provide a slice of keys they want to allow in-line with their Stream definition. Issues raised with this yesterday:

  1. The specification may need to have the work "parameter" removed from the view configuration of the stream section. It becomes less precise, but would open up this type of solution.
  2. Ideally, the TC member who will review our implementation should be asked if they think this change is appropriate. We should look for their sign-off on this.

@MrAlias
Copy link
Contributor Author

MrAlias commented Aug 31, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Part of OpenTelemetry Metrics pkg:SDK Related to an SDK package
Projects
No open projects
2 participants