Implement PoC of MeasurementProcessor proposal #4642
Conversation
| processor = BaggageMeasurementProcessor() | ||
|
|
||
| # Add only specific baggage keys | ||
| processor = BaggageMeasurementProcessor(baggage_keys=["user.id", "trace.id"]) |
There was a problem hiding this comment.
using trace.id here is misleading as there is no good use cases for it. To achieve correlation with Distributed Tracing, Exemplars can (should) be leveraged!
There was a problem hiding this comment.
"user.id" is a good use-case.
I may also suggest "synthetic_request" as another - its used in OTel Demo
(Its not a standard or anything, but something already in demo)
|
|
||
| ```python | ||
| processor = StaticAttributeMeasurementProcessor({ | ||
| "environment": "production", |
There was a problem hiding this comment.
I am not sure if this is a good idea. For StaticAttributes for all measurements, couldn't we model it as Resource Attributes. If applicable to a subset, then maybe use Meter attributes?
|
|
||
| #### 4. ValueRangeMeasurementProcessor | ||
|
|
||
| Drops measurements outside a specified value range. |
There was a problem hiding this comment.
while this looks valid, I am not sure if its common to drop measurements based on value....
May I suggest another one which seems to me like a very valid use case - collapsing certain attributes to lower the cardinality. Described here: open-telemetry/opentelemetry-dotnet#6332
| ) -> None: | ||
| # Example: Add timestamp attribute | ||
| new_attributes = dict(measurement.attributes or {}) | ||
| new_attributes["processed_at"] = str(int(time.time())) |
There was a problem hiding this comment.
I don't think we should show example of adding timestamp as a Metric attribute - is there any valid use-case for it?
| next_processor(modified_measurement) | ||
|
|
||
| # Unit conversion processor | ||
| class MetersToFeetProcessor(MeasurementProcessor): |
There was a problem hiding this comment.
I like this! A simpler one would be to do sec->msec.
Qn : Should a view also be added to change the unit to account for this processor?
| else: | ||
| next_processor(measurement) | ||
|
|
||
| # Sampling processor |
There was a problem hiding this comment.
not sure of the utility of this for Metrics!
| try: | ||
| request_counter.add(1, {"endpoint": "/api/users", "method": "GET"}) | ||
| response_time_histogram.record( | ||
| 0.150, {"endpoint": "/api/users", "status": "200"} |
There was a problem hiding this comment.
nit: if status_code, should it be 200 and not "200"?
Description
This PR is a Proof of Concept of the MeasurementProcessor concept for the OpenTelemetry Python SDK, following the design specified in OpenTelemetry Specification PR #4318.
MeasurementProcessor enables powerful measurement processing capabilities including:
The implementation uses a chain-of-responsibility pattern where each processor decides whether to pass, modify, or drop measurements by calling (or not calling) the next processor in the chain.
Key components added:
MeasurementProcessorabstract interfaceMeasurementProcessorChainfor managing processor executionMeterProvider,SdkConfiguration, andMeasurementConsumerUsage:
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
I'm not a Python expert, and this is just a PoC. Therefore, no extensive testing has been conducted.
There are some tests that can be run with the following bash command:
You can also see and run the example:
Does This PR Require a Contrib Repo Change?
Checklist: