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

stream: update stream configuration to include/exclude #118

Merged

Conversation

codeboten
Copy link
Contributor

@codeboten codeboten commented Sep 9, 2024

This follows open-telemetry/opentelemetry-specification#4188 to add support to exclude attribute keys from stream configuration.

Fixes #112

This follows open-telemetry/opentelemetry-specification#4188 to add support
to exclude attribute keys from stream configuration.

Signed-off-by: Alex Boten <[email protected]>
@codeboten
Copy link
Contributor Author

Will mark ready for review once the spec change is merged

@jack-berg jack-berg mentioned this pull request Sep 17, 2024
@codeboten codeboten marked this pull request as ready for review September 18, 2024 15:44
@codeboten codeboten requested a review from a team September 18, 2024 15:44
@codeboten
Copy link
Contributor Author

spec has merged, this is now ready for review

Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: Alex Boten <[email protected]>
@@ -262,6 +262,7 @@ meter_provider:
included:
- key1
- key2
# Configure resource attributes to be excluded, in this example attribute service.attr1.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Configure resource attributes to be excluded, in this example attribute service.attr1.
# Configure resource attributes to be excluded, in this example attribute key3.

This brings up a good point. It seems like this needs to be either a list to include, OR a list to exclude, but not both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does it? I would expect this to be similar to how resource attributes can be included or excluded. The example in the kitchen sink shows an include of service.* and an exclude of service.attr1

Copy link
Member

Choose a reason for hiding this comment

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

Not sure how SDKs would handle the presence of both include and exclude lists. I know java (and probably go) can handle.

@codeboten codeboten requested a review from a team as a code owner September 19, 2024 14:42
@codeboten codeboten merged commit e21ffdb into open-telemetry:main Sep 19, 2024
11 checks passed
@codeboten codeboten deleted the codeboten/exclude-attribute-keys branch September 19, 2024 16:42
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.

Should attribute_keys in stream configuration support exclude lists?
2 participants