-
Notifications
You must be signed in to change notification settings - Fork 952
Add support for excluding attributes in metrics View #5332
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
base: main
Are you sure you want to change the base?
Changes from all commits
87576cc
dc14a6d
15256cc
65a652b
498b25b
39ae4d8
3d15f96
512dace
e5e2f3c
ac4e44a
e70b832
7b4046e
87d042c
aa4e89b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| `opentelemetry-sdk`: add support for parameter `exclude_attribute_keys` in `View` |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -239,12 +239,10 @@ def _create_view(config: ViewConfig) -> View: | |||||
| ) | ||||||
|
|
||||||
| attribute_keys: set[str] | None = None | ||||||
| exclude_attribute_keys: set[str] | None = None | ||||||
| if stream.attribute_keys is not None: | ||||||
| if stream.attribute_keys.excluded: | ||||||
| _logger.warning( | ||||||
| "attribute_keys.excluded is not supported by the Python SDK View; " | ||||||
| "the exclusion list will be ignored." | ||||||
| ) | ||||||
| if stream.attribute_keys.excluded is not None: | ||||||
| exclude_attribute_keys = set(stream.attribute_keys.excluded) | ||||||
|
saurabh-saraswat marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| if stream.attribute_keys.included is not None: | ||||||
| attribute_keys = set(stream.attribute_keys.included) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
|
|
@@ -263,6 +261,7 @@ def _create_view(config: ViewConfig) -> View: | |||||
| description=stream.description, | ||||||
| attribute_keys=attribute_keys, | ||||||
| aggregation=aggregation, | ||||||
| exclude_attribute_keys=exclude_attribute_keys, | ||||||
| ) | ||||||
|
|
||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -98,6 +98,21 @@ def consume_measurement( | |
| else: | ||
| attributes = {} | ||
|
|
||
| if self._view._exclude_attribute_keys: | ||
| attributes = { | ||
| key: value | ||
| for key, value in attributes.items() | ||
| if key not in self._view._exclude_attribute_keys | ||
| } | ||
|
Comment on lines
+101
to
+106
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can be moved as an |
||
|
|
||
| if self._view._attribute_keys is not None or self._view._exclude_attribute_keys: | ||
| measurement = Measurement( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we added this here? |
||
| value=measurement.value, | ||
| time_unix_nano=measurement.time_unix_nano, | ||
| instrument=measurement.instrument, | ||
| context=measurement.context, | ||
| attributes=attributes, | ||
| ) | ||
| aggr_key = frozenset(attributes.items()) | ||
|
|
||
| if aggr_key not in self._attributes_aggregation: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -88,6 +88,12 @@ class View: | |
| instrument_unit: This is an instrument matching attribute: the unit the | ||
| instrument must have to match the view. | ||
|
|
||
| exclude_attribute_keys: This is a metric stream customizing attribute: this is | ||
| a set of attribute keys. If not `None` then measurement attributes whose | ||
| keys are in ``exclude_attribute_keys`` will be removed before identifying | ||
| the metric stream. Applied after ``attribute_keys`` if both are provided. | ||
|
|
||
|
|
||
| This class is not intended to be subclassed by the user. | ||
| """ | ||
|
|
||
|
|
@@ -109,6 +115,7 @@ def __init__( | |
| ] | ||
| | None = None, | ||
| instrument_unit: str | None = None, | ||
| exclude_attribute_keys: set[str] | None = None, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we make the type hint on |
||
| ): | ||
| if ( | ||
| instrument_type | ||
|
|
@@ -147,11 +154,18 @@ def __init__( | |
| self._meter_schema_url = meter_schema_url | ||
|
|
||
| self._description = description | ||
| self._attribute_keys = attribute_keys | ||
| self._attribute_keys = ( | ||
| frozenset(attribute_keys) if attribute_keys is not None else None | ||
| ) | ||
| self._aggregation = aggregation or self._default_aggregation | ||
| self._exemplar_reservoir_factory = ( | ||
| exemplar_reservoir_factory or _default_reservoir_factory | ||
| ) | ||
| self._exclude_attribute_keys = ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to add a check in the constructor to validate that attribute keys and excluded attribute keys are disjoint. |
||
| frozenset(exclude_attribute_keys) | ||
| if exclude_attribute_keys is not None | ||
| else None | ||
| ) | ||
|
|
||
| # pylint: disable=too-many-return-statements | ||
| # pylint: disable=too-many-branches | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.