Conversation
…ded to python-zocalo
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
| if isinstance(message, dict): | ||
| environment = message.get("environment", {}) | ||
| if isinstance(environment, dict): | ||
| recipe_id = environment.get("ID") |
There was a problem hiding this comment.
rw is guaranteed to be initialised here, line 105 accesses to get this same data, so probably can be replaced by
recipe_id = rw.environment.get("ID")
or, to taste,
if recipe_id := rw.environment.get("ID"):
| span.add_event( | ||
| "recipe.id_extracted", attributes={"recipe_id": recipe_id} | ||
| ) |
There was a problem hiding this comment.
I think we don't need this?
| log_extra = { | ||
| "span_id": span_id, | ||
| "trace_id": trace_id, | ||
| } | ||
|
|
||
| if recipe_id: | ||
| log_extra["recipe_id"] = recipe_id |
There was a problem hiding this comment.
this currently appears vestigial; did you mean to use log_extender?
There was a problem hiding this comment.
it looks like the easiest way to attach this is via log_extender. possibly https://docs.python.org/3/library/contextlib.html#contextlib.ExitStack helps with the possibly-not-present-multiple context managers
| license = { text = "BSD-3-Clause" } | ||
| requires-python = ">=3.10" | ||
| dependencies = ["bidict", "pika", "setuptools", "stomp-py>=7"] | ||
| dependencies = ["zocalo","marshmallow","bidict", "pika", "setuptools", "stomp-py>=7", "opentelemetry-api==1.20.0", "opentelemetry-sdk==1.20.0", "opentelemetry-exporter-otlp-proto-http==1.20.0" ] |
There was a problem hiding this comment.
remove zocalo and marshmallow
| # Configure OTELTracing if configuration is available | ||
| otel_config = ( | ||
| self.config.opentelemetry | ||
| if self.config and hasattr(self.config, "opentelemetry") |
There was a problem hiding this comment.
Looks like in https://github.com/DiamondLightSource/python-zocalo/blob/b3e3ca4addba6e61fab0bef2fcb825a49e73938a/src/zocalo/configuration/__init__.py#L150 that the name set is _opentelemetry - assuming the self.config object is what we expect
| ) | ||
|
|
||
| if otel_config: | ||
| if "endpoint" not in otel_config: |
There was a problem hiding this comment.
If you have already validated these in the configuration.OTEL plugin then you do not need to do so again; also - if you have asked to configure but provided wrong values, this is an error
| except Exception as e: | ||
| # Continue without tracing if configuration fails | ||
| self.log.warning( | ||
| "Failed to configure OpenTelemetry tracing: %s", str(e) | ||
| ) |
There was a problem hiding this comment.
if the user has explicitly requested opentelemetry then failing to configure it should probably be an error
2 participants
Added OTEL Tracing to zocalo service