Add OpenTelemetry instrumentation #59
Conversation
andmat900
requested review from
t-persson and
fredjn
and removed request for
a team
andmat900
force-pushed
the
20240402_opentelemetry
branch
from
c12ae03 to
e187c17
Compare
| span_name = "start_execution_space" | ||
| with self.tracer.start_as_current_span(span_name) as span: | ||
| span.set_attribute("executor_id", executor["id"]) | ||
| span.set_attribute("request", dumps(request, indent=4)) |
There was a problem hiding this comment.
Is indent=4 necessary? It increases the request size to the open telemetry collector
There was a problem hiding this comment.
The problem is that it isn't readable. At least in Jaeger where it is shown as a simple text blob.
There was a problem hiding this comment.
The indent argument is now removed.
| # OpenTelemetry context needs to be retrieved here: | ||
| # the subsuite is running in a separate process |
There was a problem hiding this comment.
Is this comment still valid?
There was a problem hiding this comment.
The comment is relevant, I have clarified it in the latest update.
| def get_current_context() -> opentelemetry.context.context.Context: | ||
| """Get current context (propagated via environment variable OTEL_CONTEXT).""" | ||
| carrier = {} | ||
| LOGGER.info("Current OpenTelemetry context env: %s", os.environ.get("OTEL_CONTEXT")) | ||
| for kv in os.environ.get("OTEL_CONTEXT", "").split(","): | ||
| if kv: | ||
| k, v = kv.split("=", 1) | ||
| carrier[k] = v | ||
| ctx = opentelemetry.propagate.extract(carrier) | ||
| LOGGER.info("Current OpenTelemetry context %s", ctx) | ||
| return ctx |
There was a problem hiding this comment.
We do something similar to this in the ETOS library where we utilize the built-in textmap propagator.
Called here: https://github.com/eiffel-community/etos-library/blob/main/src/etos_lib/eiffel/subscriber.py#L104-L106
Extracted here: https://github.com/eiffel-community/etos-library/blob/main/src/etos_lib/eiffel/subscriber.py#L37-L52
I believe that the one in ETOS library is simpler.
There was a problem hiding this comment.
In the latest update I have made it more idiomatic. It does not make the code simpler, but it may look better.
| timeout = time.time() + self.etos.debug.default_test_result_timeout | ||
| try: | ||
| while time.time() < timeout: | ||
| time.sleep(10) |
There was a problem hiding this comment.
Isn't doing mandatory sleeping 10 a bit suboptimal? Couldn't we just have shorter sleep inside the 'not self.started'-loop?
There was a problem hiding this comment.
I'd avoid changing it here. This is old code, I just had to change the indent due to opentelemetry span recording.
There was a problem hiding this comment.
That sleep is there so that we do not spam the event repository too much. It is needed for both the if not self.started and further down if self.finished is False.
This loop is for waiting for the test runner to start and then to finish.
andmat900
force-pushed
the
20240402_opentelemetry
branch
4 times, most recently
from
f9fd9aa to
84f150b
Compare
andmat900
force-pushed
the
20240402_opentelemetry
branch
from
84f150b to
11b187c
Compare
andmat900
force-pushed
the
20240402_opentelemetry
branch
from
11b187c to
ebacc99
Compare
Applicable Issues
Description of the Change
This change adds OpenTelemetry instrumentation to etos-suite-runner.
Alternate Designs
Possible Drawbacks
Sign-off
Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
Signed-off-by: Andrei Matveyeu, [email protected]