Add OpenTelemetry tracing #96
Conversation
andmat900
requested review from
t-persson and
fredjn
and removed request for
a team
| otel_span.set_attribute("request.headers", json.dumps(headers, indent=4)) | ||
| otel_span.set_attribute("request.body", json.dumps(execution_spaces, indent=4)) |
There was a problem hiding this comment.
Should we really indent the JSON here? It makes the request to the OTEL collector larger.
This comment applies to all indent=4 dumps
There was a problem hiding this comment.
If this isn't done, the JSON is shown in Jaeger as a string without pretty formatting.
There was a problem hiding this comment.
For me that's OK. I usually pipe JSON into jq for prettifying. The problem I have with posting pretty-json is that it makes the request a lot larger for something that, to me, does not give that much of a benefit.
@fredjn can chime in on which way he leans on this :)
There was a problem hiding this comment.
I'm leaning towards keeping the amount of data down, and If we feel it's a problem to re-format the json (when/if we inspect it) we can re-evaluate and refactor.
There was a problem hiding this comment.
Pretty formatting is now removed.
| otel_span.set_attribute("request.headers", json.dumps(headers, indent=4)) | ||
| otel_span.set_attribute("request.body", json.dumps(execution_spaces, indent=4)) |
There was a problem hiding this comment.
For me that's OK. I usually pipe JSON into jq for prettifying. The problem I have with posting pretty-json is that it makes the request a lot larger for something that, to me, does not give that much of a benefit.
@fredjn can chime in on which way he leans on this :)
| @@ -114,8 +123,8 @@ def checkin(self, log_area: LogArea) -> None: | |||
| headers = {"X-ETOS-ID": self.identifier} | |||
| TraceContextTextMapPropagator().inject(headers) | |||
| otel_span = opentelemetry.trace.get_current_span() | |||
| otel_span.set_attribute("http.request.host", host) | |||
| otel_span.set_attribute("http.request.body", json.dumps(log_areas, indent=4)) | |||
| otel_span.set_attribute(SpanAttributes.URL_FULL, host) | |||
There was a problem hiding this comment.
Perhaps rename to span to keep naming consistent?
andmat900
force-pushed
the
20240402_opentelemetry
branch
from
b23aa80 to
e53b984
Compare
andmat900
force-pushed
the
20240402_opentelemetry
branch
from
b16540e to
8c5237d
Compare
Applicable Issues
Description of the Change
This change adds OpenTelemetry instrumentation to etos-environment-provider.
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]