[TEST PR only] WIP Span: add attribute, event, link setter to the API #1
Conversation
alban
force-pushed
the
alban/attribute
branch
7 times, most recently
from
4414c61 to
ecf1bee
Compare
| @@ -102,6 +104,38 @@ def get_context(self) -> 'SpanContext': | |||
| A :class:`.SpanContext` with a copy of this span's immutable state. | |||
| """ | |||
|
|
|||
| def set_attribute(self: 'Span', | |||
There was a problem hiding this comment.
I don't see the type annotation used for self parameters anywhere in the file.
There was a problem hiding this comment.
They do that in the subclass in:
opentelemetry-sdk/src/opentelemetry/sdk/trace/init.py
Not sure if the inconsistency is on purpose
| Sets a single Attribute with the key and value passed as arguments. | ||
| """ | ||
|
|
||
| def add_event(self: 'Span', |
| TODO: AddLazyEvent: what is that? | ||
| """ | ||
|
|
||
| def add_link(self: 'Span', |
| Adds a single Event with the name and, optionally, attributes passed | ||
| as arguments. | ||
|
|
||
| TODO: AddLazyEvent: what is that? |
There was a problem hiding this comment.
My understanding of the lazy stuff was that it is basically done kind of a builder pattern. So you basically have an object that is not yet an event, and it has a bunch of setters for each property. After you are done with setting it all up, you call some kind of a commit function that creates the event from the object. That way you don't need to pass everything up front to a single function. I'm not sure how this kind of pattern gets implemented in python. This kind of pattern sometimes appears in C, for example.
There was a problem hiding this comment.
I see... I don't see this used in OT-Python, so I'll remove it.
| @@ -114,6 +114,34 @@ def test_start_span_explicit(self): | |||
| self.assertIs(tracer.get_current_span(), root) | |||
| self.assertIsNotNone(child.end_time) | |||
|
|
|||
| def test_span_members(self): | |||
There was a problem hiding this comment.
Does this test check anything? Like if the attribute was overwritten properly?
There was a problem hiding this comment.
No, it does not test. I think it can be added later when the attributes are actually used (when serialising?). I will add a comment in the commitmsg though.
Basic tests that exercise the API are added as well. However it does not actually test that the attributes, events and links are written properly because there is no getter. Those tests can be added later when the attributes are actually used. The spec mentions additional APIs for lazy initialization (AddLazyEvent, AddLazyLink). Those are not added in this patch because OT-Python does not expose the Event type (currently defined in the SDK only).
Co-Authored-By: Chris Kleinknecht <libc@google.com>
Co-Authored-By: Chris Kleinknecht <libc@google.com>
Even though the public API does not expose getters, we can access the span members directly in Python.
Symptoms: ``` /home/travis/build/kinvolk/opentelemetry-python/opentelemetry-api/src/opentelemetry/trace/__init__.py:docstring of opentelemetry.trace.Span.set_attribute:3: WARNING: 'any' reference target not found: Attribute 298/home/travis/build/kinvolk/opentelemetry-python/opentelemetry-api/src/opentelemetry/trace/__init__.py:docstring of opentelemetry.trace.Span.add_link:3: WARNING: 'any' reference target not found: Link 299 ```
Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>
Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>
Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>
Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>
Symptoms: ``` opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py:249:4: W0221: Parameters differ from overridden 'add_link' method (arguments-differ) ```
|
My test is done. Closing. The PR upstream (83) was merged. |
- use Args: notation in documentation - remove license boilerplate from empty file - use "not x" instead of "len(x) == 0" - rename logger
mauriciovasquezbernal
force-pushed
the
master
branch
5 times, most recently
from
00a6d5b to
4e551ba
Compare
No description provided.