-
Notifications
You must be signed in to change notification settings - Fork 8
Added OTEL tracing #196
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
base: main
Are you sure you want to change the base?
Added OTEL tracing #196
Changes from all commits
722bf3e
52cb04d
cc9ee12
f7cc658
8b2a2f1
0686e28
2d9e21c
3a5283a
4b999f1
4b86715
9c13d07
3e0b902
d446e80
16b0e10
7ad857f
468f940
7aae664
902a7df
9e5adb7
1d7457e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,8 @@ | |
| from collections.abc import Callable | ||
| from typing import Any | ||
|
|
||
| from opentelemetry import trace | ||
|
|
||
| from workflows.recipe.recipe import Recipe | ||
| from workflows.recipe.validate import validate_recipe | ||
| from workflows.recipe.wrapper import RecipeWrapper | ||
|
|
@@ -69,6 +71,37 @@ def unwrap_recipe(header, message): | |
| message = mangle_for_receiving(message) | ||
| if header.get("workflows-recipe") in {True, "True", "true", 1}: | ||
| rw = RecipeWrapper(message=message, transport=transport_layer) | ||
|
|
||
| # Extract recipe_id on the current span | ||
| span = trace.get_current_span() | ||
| recipe_id = None | ||
|
|
||
| # Extract recipe ID from environment | ||
| if isinstance(message, dict): | ||
| environment = message.get("environment", {}) | ||
| if isinstance(environment, dict): | ||
| recipe_id = environment.get("ID") | ||
|
Comment on lines
+80
to
+83
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
or, to taste,
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
|
|
||
| if recipe_id: | ||
| span.set_attribute("recipe_id", recipe_id) | ||
| span.add_event( | ||
| "recipe.id_extracted", attributes={"recipe_id": recipe_id} | ||
| ) | ||
|
Comment on lines
+87
to
+89
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we don't need this?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed- fixed
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed-fixed |
||
|
|
||
| # Extract span_id and trace_id for logging | ||
| span_context = span.get_span_context() | ||
| if span_context and span_context.is_valid: | ||
| span_id = format(span_context.span_id, "016x") | ||
| trace_id = format(span_context.trace_id, "032x") | ||
|
|
||
| log_extra = { | ||
| "span_id": span_id, | ||
| "trace_id": trace_id, | ||
| } | ||
|
|
||
| if recipe_id: | ||
| log_extra["recipe_id"] = recipe_id | ||
|
Comment on lines
+97
to
+103
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this currently appears vestigial; did you mean to use log_extender?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
|
|
||
| if log_extender and rw.environment and rw.environment.get("ID"): | ||
| with log_extender("recipe_ID", rw.environment["ID"]): | ||
| return callback(rw, header, message.get("payload")) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,8 +9,15 @@ | |
| import time | ||
| from typing import Any | ||
|
|
||
| from opentelemetry import trace | ||
| from opentelemetry.exporter.otlp.proto.http.trace_exporter import OTLPSpanExporter | ||
| from opentelemetry.sdk.resources import SERVICE_NAME, Resource | ||
| from opentelemetry.sdk.trace import TracerProvider | ||
| from opentelemetry.sdk.trace.export import BatchSpanProcessor | ||
|
|
||
| import workflows | ||
| import workflows.logging | ||
| from workflows.transport.middleware.otel_tracing import OTELTracingMiddleware | ||
|
|
||
|
|
||
| class Status(enum.Enum): | ||
|
|
@@ -185,6 +192,56 @@ def start_transport(self): | |
| self.transport.subscription_callback_set_intercept( | ||
| self._transport_interceptor | ||
| ) | ||
| try: | ||
| # Configure OTELTracing if configuration is available | ||
| otel_config = ( | ||
| self.config.opentelemetry | ||
| if self.config and hasattr(self.config, "opentelemetry") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like in https://github.com/DiamondLightSource/python-zocalo/blob/b3e3ca4addba6e61fab0bef2fcb825a49e73938a/src/zocalo/configuration/__init__.py#L150 that the name set is |
||
| else None | ||
| ) | ||
|
|
||
| if otel_config: | ||
| if "endpoint" not in otel_config: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| self.log.warning( | ||
| "Missing required OTEL configuration field `endpoint`." | ||
| ) | ||
|
|
||
| if "timeout" not in otel_config: | ||
| self.log.warning( | ||
| "Missing optional OTEL configuration field `timout`. Will default to 10 seconds. " | ||
davidigandan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ) | ||
|
|
||
| # Configure OTELTracing | ||
| resource = Resource.create( | ||
| { | ||
| SERVICE_NAME: self._service_name, | ||
| } | ||
| ) | ||
|
|
||
| self.log.debug("Configuring OTELTracing") | ||
| provider = TracerProvider(resource=resource) | ||
| trace.set_tracer_provider(provider) | ||
|
|
||
| # Configure BatchProcessor and OTLPSpanExporter using config values | ||
| otlp_exporter = OTLPSpanExporter( | ||
| endpoint=otel_config["endpoint"], | ||
| timeout=otel_config.get("timeout", 10), | ||
| ) | ||
| span_processor = BatchSpanProcessor(otlp_exporter) | ||
| provider.add_span_processor(span_processor) | ||
|
|
||
| # Add OTELTracingMiddleware to the transport layer | ||
| tracer = trace.get_tracer(__name__) | ||
| otel_middleware = OTELTracingMiddleware( | ||
| tracer, service_name=self._service_name | ||
| ) | ||
| self._transport.add_middleware(otel_middleware) | ||
| except Exception as e: | ||
| # Continue without tracing if configuration fails | ||
| self.log.warning( | ||
| "Failed to configure OpenTelemetry tracing: %s", str(e) | ||
| ) | ||
|
Comment on lines
+239
to
+243
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if the user has explicitly requested opentelemetry then failing to configure it should probably be an error |
||
|
|
||
| metrics = self._environment.get("metrics") | ||
| if metrics: | ||
| import prometheus_client | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import functools | ||
| from collections.abc import Callable | ||
|
|
||
| from opentelemetry import trace | ||
| from opentelemetry.propagate import extract | ||
|
|
||
| from workflows.transport.middleware import BaseTransportMiddleware | ||
|
|
||
|
|
||
| class OTELTracingMiddleware(BaseTransportMiddleware): | ||
| def __init__(self, tracer: trace.Tracer, service_name: str): | ||
| self.tracer = tracer | ||
| self.service_name = service_name | ||
|
|
||
| def subscribe(self, call_next: Callable, channel, callback, **kwargs) -> int: | ||
| @functools.wraps(callback) | ||
| def wrapped_callback(header, message): | ||
| # Extract trace context from message headers | ||
| ctx = extract(header) if header else None | ||
|
|
||
| # Start a new span with the extracted context | ||
| with self.tracer.start_as_current_span( | ||
| "transport.subscribe", context=ctx | ||
| ) as span: | ||
| span.set_attribute("service_name", self.service_name) | ||
| span.set_attribute("channel", channel) | ||
|
|
||
| # Call the original callback | ||
| return callback(header, message) | ||
|
|
||
| # Call the next middleware with the wrapped callback | ||
| return call_next(channel, wrapped_callback, **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove zocalo and marshmallow