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

Add fields property #1374

Merged
merged 9 commits into from
Dec 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ env:
# Otherwise, set variable to the commit of your branch on
# opentelemetry-python-contrib which is compatible with these Core repo
# changes.
CONTRIB_REPO_SHA: bcec49cf2eccf8da66c9e63b9836ea8a20516efc
CONTRIB_REPO_SHA: e1d4eeb951694afebb1767b4ea8f593753d894e3

jobs:
build:
Expand Down
12 changes: 9 additions & 3 deletions opentelemetry-api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,24 @@

## Unreleased

- Add `fields` to propagators
([#1374](https://github.com/open-telemetry/opentelemetry-python/pull/1374))

## Version 0.16b0

Released 2020-11-25

- Add optional parameter to `record_exception` method ([#1314](https://github.com/open-telemetry/opentelemetry-python/pull/1314))
- Add pickle support to SpanContext class ([#1380](https://github.com/open-telemetry/opentelemetry-python/pull/1380))
- Add optional parameter to `record_exception` method
([#1314](https://github.com/open-telemetry/opentelemetry-python/pull/1314))
- Add pickle support to SpanContext class
([#1380](https://github.com/open-telemetry/opentelemetry-python/pull/1380))

## Version 0.15b0

Released 2020-11-02

- Updating status codes to adhere to specs ([#1282](https://github.com/open-telemetry/opentelemetry-python/pull/1282))
- Updating status codes to adhere to specs
([#1282](https://github.com/open-telemetry/opentelemetry-python/pull/1282))

## Version 0.14b0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@ def inject(
baggage_string = _format_baggage(baggage_entries)
set_in_carrier(carrier, self._BAGGAGE_HEADER_NAME, baggage_string)

@property
def fields(self) -> typing.Set[str]:
"""Returns a set with the fields set in `inject`."""
return {self._BAGGAGE_HEADER_NAME}


def _format_baggage(baggage_entries: typing.Mapping[str, object]) -> str:
return ",".join(
Expand Down
15 changes: 15 additions & 0 deletions opentelemetry-api/src/opentelemetry/propagators/composite.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,18 @@ def inject(
"""
for propagator in self._propagators:
propagator.inject(set_in_carrier, carrier, context)

@property
def fields(self) -> typing.Set[str]:
"""Returns a set with the fields set in `inject`.

See
`opentelemetry.trace.propagation.textmap.TextMapPropagator.fields`
"""
composite_fields = set()

for propagator in self._propagators:
for field in propagator.fields:
composite_fields.add(field)
Copy link
Contributor

Choose a reason for hiding this comment

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

is the behaviour for the composite propagator specified in the spec?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it is, I tried to replicate the Go implementation with one particular difference. In this implementation a set is returned instead of a list. There is no defined order for these fields (these fields may be called in inject in an arbitrary order and even if this order is constant it has no relation with the fields themselves), so an ordered sequence like a list makes no sense. Also, (if I understand correctly) the intention of this attribute is to allow the user to tell if a certain field is also used in inject. That is an in operation that has a better (O(1)) performance in a set than in a list (O(N)).


return composite_fields
13 changes: 13 additions & 0 deletions opentelemetry-api/src/opentelemetry/trace/propagation/textmap.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,3 +129,16 @@ def inject(
context if not set.

"""

@property
@abc.abstractmethod
def fields(self) -> typing.Set[str]:
"""
Gets the fields set in the carrier by the `inject` method.

If the carrier is reused, its fields that correspond with the ones
present in this attribute should be deleted before calling `inject`.

Returns:
A set with the fields set in `inject`.
"""
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,15 @@ def inject(
carrier, self._TRACESTATE_HEADER_NAME, tracestate_string
)

@property
def fields(self) -> typing.Set[str]:
"""Returns a set with the fields set in `inject`.

See
`opentelemetry.trace.propagation.textmap.TextMapPropagator.fields`
"""
return {self._TRACEPARENT_HEADER_NAME, self._TRACESTATE_HEADER_NAME}

ocelotl marked this conversation as resolved.
Show resolved Hide resolved

def _parse_tracestate(header_list: typing.List[str]) -> trace.TraceState:
"""Parse one or more w3c tracestate header into a TraceState.
Expand Down
17 changes: 16 additions & 1 deletion opentelemetry-api/tests/baggage/test_baggage_propagation.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
import typing
import unittest
from unittest.mock import Mock, patch

from opentelemetry import baggage
from opentelemetry.baggage.propagation import BaggagePropagator
Expand Down Expand Up @@ -142,3 +142,18 @@ def test_inject_non_string_values(self):
self.assertIn("key1=True", output)
self.assertIn("key2=123", output)
self.assertIn("key3=123.567", output)

@patch("opentelemetry.baggage.propagation.baggage")
@patch("opentelemetry.baggage.propagation._format_baggage")
def test_fields(self, mock_format_baggage, mock_baggage):

mock_set_in_carrier = Mock()

self.propagator.inject(mock_set_in_carrier, {})

inject_fields = set()

for mock_call in mock_set_in_carrier.mock_calls:
inject_fields.add(mock_call[1][1])

self.assertEqual(inject_fields, self.propagator.fields)
35 changes: 33 additions & 2 deletions opentelemetry-api/tests/propagators/test_composite.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ def get_as_list(dict_object, key):
def mock_inject(name, value="data"):
def wrapped(setter, carrier=None, context=None):
carrier[name] = value
setter({}, "inject_field_{}_0".format(name), None)
setter({}, "inject_field_{}_1".format(name), None)

return wrapped

Expand All @@ -39,18 +41,27 @@ def wrapped(getter, carrier=None, context=None):
return wrapped


def mock_fields(name):
return {"inject_field_{}_0".format(name), "inject_field_{}_1".format(name)}


class TestCompositePropagator(unittest.TestCase):
@classmethod
def setUpClass(cls):
cls.mock_propagator_0 = Mock(
inject=mock_inject("mock-0"), extract=mock_extract("mock-0")
inject=mock_inject("mock-0"),
extract=mock_extract("mock-0"),
fields=mock_fields("mock-0"),
)
cls.mock_propagator_1 = Mock(
inject=mock_inject("mock-1"), extract=mock_extract("mock-1")
inject=mock_inject("mock-1"),
extract=mock_extract("mock-1"),
fields=mock_fields("mock-1"),
)
cls.mock_propagator_2 = Mock(
inject=mock_inject("mock-0", value="data2"),
extract=mock_extract("mock-0", value="context2"),
fields=mock_fields("mock-0"),
)

def test_no_propagators(self):
Expand Down Expand Up @@ -105,3 +116,23 @@ def test_multiple_propagators_same_key(self):
get_as_list, carrier=new_carrier, context={}
)
self.assertEqual(context, {"mock-0": "context2"})

def test_fields(self):
propagator = CompositeHTTPPropagator(
[
self.mock_propagator_0,
self.mock_propagator_1,
self.mock_propagator_2,
]
)

mock_set_in_carrier = Mock()

propagator.inject(mock_set_in_carrier, {})

inject_fields = set()

for mock_call in mock_set_in_carrier.mock_calls:
inject_fields.add(mock_call[1][1])

self.assertEqual(inject_fields, propagator.fields)
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import typing
import unittest
from unittest.mock import Mock, patch

from opentelemetry import trace
from opentelemetry.trace.propagation import tracecontext
Expand Down Expand Up @@ -232,7 +233,8 @@ def test_tracestate_keys(self):
carrier_getter,
{
"traceparent": [
"00-12345678901234567890123456789012-1234567890123456-00"
"00-12345678901234567890123456789012-"
"1234567890123456-00"
],
"tracestate": [tracestate_value],
},
Expand All @@ -248,3 +250,33 @@ def test_tracestate_keys(self):
self.assertEqual(
span.get_span_context().trace_state["foo-_*/bar"], "bar4"
)

@patch("opentelemetry.trace.INVALID_SPAN_CONTEXT")
@patch("opentelemetry.trace.get_current_span")
def test_fields(self, mock_get_current_span, mock_invalid_span_context):

mock_get_current_span.configure_mock(
return_value=Mock(
**{
"get_span_context.return_value": Mock(
**{
"trace_id": 1,
"span_id": 2,
"trace_flags": 3,
"trace_state": {"a": "b"},
}
)
}
)
)

mock_set_in_carrier = Mock()

FORMAT.inject(mock_set_in_carrier, {})

inject_fields = set()

for mock_call in mock_set_in_carrier.mock_calls:
inject_fields.add(mock_call[1][1])

self.assertEqual(inject_fields, FORMAT.fields)
2 changes: 2 additions & 0 deletions opentelemetry-sdk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

- Add meter reference to observers
([#1425](https://github.com/open-telemetry/opentelemetry-python/pull/1425))
- Add `fields` to propagators
([#1374](https://github.com/open-telemetry/opentelemetry-python/pull/1374))

## Version 0.16b0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,15 @@ def inject(
)
set_in_carrier(carrier, self.SAMPLED_KEY, "1" if sampled else "0")

@property
def fields(self) -> typing.Set[str]:
return {
self.TRACE_ID_KEY,
self.SPAN_ID_KEY,
self.PARENT_SPAN_ID_KEY,
self.SAMPLED_KEY,
}


def format_trace_id(trace_id: int) -> str:
"""Format the trace id according to b3 specification."""
Expand Down
20 changes: 19 additions & 1 deletion opentelemetry-sdk/tests/trace/propagation/test_b3_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
# limitations under the License.

import unittest
from unittest.mock import patch
from unittest.mock import Mock, patch

import opentelemetry.sdk.trace as trace
import opentelemetry.sdk.trace.propagation.b3_format as b3_format
Expand Down Expand Up @@ -337,3 +337,21 @@ def setter(carrier, key, value):

ctx = FORMAT.extract(CarrierGetter(), {})
FORMAT.inject(setter, {}, ctx)

def test_fields(self):
"""Make sure the fields attribute returns the fields used in inject"""

tracer = trace.TracerProvider().get_tracer("sdk_tracer_provider")

mock_set_in_carrier = Mock()

with tracer.start_as_current_span("parent"):
with tracer.start_as_current_span("child"):
FORMAT.inject(mock_set_in_carrier, {})

inject_fields = set()

for call in mock_set_in_carrier.mock_calls:
inject_fields.add(call[1][1])

self.assertEqual(FORMAT.fields, inject_fields)
8 changes: 8 additions & 0 deletions tests/util/src/opentelemetry/test/mock_textmap.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ def inject(
) -> None:
return None

@property
def fields(self):
return set()


class MockTextMapPropagator(TextMapPropagator):
"""Mock propagator for testing purposes."""
Expand Down Expand Up @@ -89,3 +93,7 @@ def inject(
set_in_carrier(
carrier, self.SPAN_ID_KEY, str(span.get_span_context().span_id)
)

@property
def fields(self):
return {self.TRACE_ID_KEY, self.SPAN_ID_KEY}