-
Notifications
You must be signed in to change notification settings - Fork 815
Add AlwaysRecordSampler #4823
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?
Add AlwaysRecordSampler #4823
Changes from all commits
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,81 @@ | ||||||||||||||||||||||||||||||||||
| # Copyright The OpenTelemetry Authors | ||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||||||||||||||||||||||||||||||||||
| # you may not use this file except in compliance with the License. | ||||||||||||||||||||||||||||||||||
| # You may obtain a copy of the License at | ||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||
| # Unless required by applicable law or agreed to in writing, software | ||||||||||||||||||||||||||||||||||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||||||||||||||||||||||||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||||||||||||||||||||||||
| # See the License for the specific language governing permissions and | ||||||||||||||||||||||||||||||||||
| # limitations under the License. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| from typing import Optional, Sequence | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| from opentelemetry.context import Context | ||||||||||||||||||||||||||||||||||
| from opentelemetry.sdk.trace.sampling import Decision, Sampler, SamplingResult | ||||||||||||||||||||||||||||||||||
| from opentelemetry.trace import Link, SpanKind | ||||||||||||||||||||||||||||||||||
| from opentelemetry.trace.span import TraceState | ||||||||||||||||||||||||||||||||||
| from opentelemetry.util.types import Attributes | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| class AlwaysRecordSampler(Sampler): | ||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||
| This sampler will return the sampling result of the provided `_root_sampler`, unless the | ||||||||||||||||||||||||||||||||||
| sampling result contains the sampling decision `Decision.DROP`, in which case, a | ||||||||||||||||||||||||||||||||||
| new sampling result will be returned that is functionally equivalent to the original, except that | ||||||||||||||||||||||||||||||||||
| it contains the sampling decision `SamplingDecision.RECORD_ONLY`. This ensures that all | ||||||||||||||||||||||||||||||||||
|
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
|
||||||||||||||||||||||||||||||||||
| spans are recorded, with no change to sampling. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| The intended use case of this sampler is to provide a means of sending all spans to a | ||||||||||||||||||||||||||||||||||
| processor without having an impact on the sampling rate. This may be desirable if a user wishes | ||||||||||||||||||||||||||||||||||
| to count or otherwise measure all spans produced in a service, without incurring the cost of 100% | ||||||||||||||||||||||||||||||||||
| sampling. | ||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| _root_sampler: Sampler | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def __init__(self, root_sampler: Sampler): | ||||||||||||||||||||||||||||||||||
| if not root_sampler: | ||||||||||||||||||||||||||||||||||
|
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
|
||||||||||||||||||||||||||||||||||
| raise ValueError("root_sampler must not be None") | ||||||||||||||||||||||||||||||||||
| self._root_sampler = root_sampler | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def should_sample( | ||||||||||||||||||||||||||||||||||
| self, | ||||||||||||||||||||||||||||||||||
| parent_context: Optional["Context"], | ||||||||||||||||||||||||||||||||||
| trace_id: int, | ||||||||||||||||||||||||||||||||||
| name: str, | ||||||||||||||||||||||||||||||||||
| kind: Optional[SpanKind] = None, | ||||||||||||||||||||||||||||||||||
| attributes: Attributes = None, | ||||||||||||||||||||||||||||||||||
| links: Optional[Sequence["Link"]] = None, | ||||||||||||||||||||||||||||||||||
| trace_state: Optional["TraceState"] = None, | ||||||||||||||||||||||||||||||||||
| ) -> "SamplingResult": | ||||||||||||||||||||||||||||||||||
| result: SamplingResult = self._root_sampler.should_sample( | ||||||||||||||||||||||||||||||||||
| parent_context, | ||||||||||||||||||||||||||||||||||
| trace_id, | ||||||||||||||||||||||||||||||||||
| name, | ||||||||||||||||||||||||||||||||||
| kind, | ||||||||||||||||||||||||||||||||||
| attributes, | ||||||||||||||||||||||||||||||||||
| links, | ||||||||||||||||||||||||||||||||||
| trace_state, | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| if result.decision is Decision.DROP: | ||||||||||||||||||||||||||||||||||
| result = _wrap_result_with_record_only_result(result, attributes) | ||||||||||||||||||||||||||||||||||
| return result | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def get_description(self): | ||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||
| "AlwaysRecordSampler{" + self._root_sampler.get_description() + "}" | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+68
to
+71
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
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def _wrap_result_with_record_only_result( | ||||||||||||||||||||||||||||||||||
| result: SamplingResult, attributes: Attributes | ||||||||||||||||||||||||||||||||||
| ) -> SamplingResult: | ||||||||||||||||||||||||||||||||||
| return SamplingResult( | ||||||||||||||||||||||||||||||||||
| Decision.RECORD_ONLY, | ||||||||||||||||||||||||||||||||||
| attributes, | ||||||||||||||||||||||||||||||||||
| result.trace_state, | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+74
to
+81
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. It might make sense to in-line this since it's only called once, also is it intentional that the result.attributes are not being used?
Suggested change
|
||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,77 @@ | ||||||||
| from unittest import TestCase | ||||||||
| from unittest.mock import MagicMock | ||||||||
|
|
||||||||
| from opentelemetry.context import Context | ||||||||
| from opentelemetry.sdk.trace._sampling_experimental import AlwaysRecordSampler | ||||||||
| from opentelemetry.sdk.trace.sampling import ( | ||||||||
| Decision, | ||||||||
| Sampler, | ||||||||
| SamplingResult, | ||||||||
| StaticSampler, | ||||||||
| ) | ||||||||
| from opentelemetry.trace import SpanKind | ||||||||
| from opentelemetry.trace.span import TraceState | ||||||||
| from opentelemetry.util.types import Attributes | ||||||||
|
|
||||||||
|
|
||||||||
| class TestAlwaysRecordSampler(TestCase): | ||||||||
| def setUp(self): | ||||||||
| self.mock_sampler: Sampler = MagicMock() | ||||||||
| self.sampler: Sampler = AlwaysRecordSampler(self.mock_sampler) | ||||||||
|
|
||||||||
| def test_get_description(self): | ||||||||
| static_sampler: Sampler = StaticSampler(Decision.DROP) | ||||||||
| test_sampler: Sampler = AlwaysRecordSampler(static_sampler) | ||||||||
| self.assertEqual( | ||||||||
| "AlwaysRecordSampler{AlwaysOffSampler}", | ||||||||
| test_sampler.get_description(), | ||||||||
| ) | ||||||||
|
|
||||||||
| def test_record_and_sample_sampling_decision(self): | ||||||||
| self.validate_should_sample( | ||||||||
| Decision.RECORD_AND_SAMPLE, Decision.RECORD_AND_SAMPLE | ||||||||
| ) | ||||||||
|
|
||||||||
| def test_record_only_sampling_decision(self): | ||||||||
| self.validate_should_sample(Decision.RECORD_ONLY, Decision.RECORD_ONLY) | ||||||||
|
|
||||||||
| def test_drop_sampling_decision(self): | ||||||||
| self.validate_should_sample(Decision.DROP, Decision.RECORD_ONLY) | ||||||||
|
|
||||||||
| def validate_should_sample( | ||||||||
| self, root_decision: Decision, expected_decision: Decision | ||||||||
| ): | ||||||||
| root_result: SamplingResult = _build_root_sampling_result( | ||||||||
| root_decision | ||||||||
| ) | ||||||||
| self.mock_sampler.should_sample.return_value = root_result | ||||||||
| actual_result: SamplingResult = self.sampler.should_sample( | ||||||||
| parent_context=Context(), | ||||||||
| trace_id=0, | ||||||||
| name="name", | ||||||||
| kind=SpanKind.CLIENT, | ||||||||
| attributes={"key": root_decision.name}, | ||||||||
| trace_state=TraceState(), | ||||||||
| ) | ||||||||
|
|
||||||||
| if root_decision == expected_decision: | ||||||||
| self.assertEqual(actual_result, root_result) | ||||||||
| self.assertEqual(actual_result.decision, root_decision) | ||||||||
| else: | ||||||||
| self.assertNotEqual(actual_result, root_result) | ||||||||
| self.assertEqual(actual_result.decision, expected_decision) | ||||||||
|
|
||||||||
| self.assertEqual(actual_result.attributes, root_result.attributes) | ||||||||
|
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. What happens if the root sampler adds or changes attributes? This also ties into how |
||||||||
| self.assertEqual(actual_result.trace_state, root_result.trace_state) | ||||||||
|
|
||||||||
|
|
||||||||
| def _build_root_sampling_result(sampling_decision: Decision): | ||||||||
| sampling_attr: Attributes = {"key": sampling_decision.name} | ||||||||
| sampling_trace_state: TraceState = TraceState() | ||||||||
| sampling_trace_state.add("key", sampling_decision.name) | ||||||||
|
Comment on lines
+70
to
+71
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.
so could this be:
Suggested change
|
||||||||
| sampling_result: SamplingResult = SamplingResult( | ||||||||
| decision=sampling_decision, | ||||||||
| attributes=sampling_attr, | ||||||||
| trace_state=sampling_trace_state, | ||||||||
| ) | ||||||||
| return sampling_result | ||||||||
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.
Might want to align the changelog style