Feat: Hatchet OTel Instrumentor #313
Conversation
| def test_push_event(hatchet: Hatchet, worker: Worker) -> None: | ||
| key = "otel:event" | ||
| payload = {"test": "test"} | ||
|
|
||
| with tracer.start_as_current_span("push_event"): | ||
| event = hatchet.event.push( | ||
| event_key=key, | ||
| payload=payload, | ||
| options=create_push_options(), | ||
| ) | ||
|
|
||
| assert event.key == key | ||
| assert event.payload == json.dumps(payload) |
There was a problem hiding this comment.
the point of these tests is to make sure the existing code in the SDK runs without any issues if we use the instrumentor (in this case with a no-op tracer). can extend these more later if they end up being valuable
There was a problem hiding this comment.
ran all these and checked out the traces in Honeycomb. E.g.:
There was a problem hiding this comment.
This basically follows how asyncio does its instrumentation - was super easy to implement, really similar to mocking in tests
|
|
||
|
|
||
| class HatchetInstrumentor(BaseInstrumentor): # type: ignore[misc] | ||
| OTEL_TRACEPARENT_KEY = "traceparent" |
There was a problem hiding this comment.
do we want to namespace this i.e. hatchet__traceparent to prevent the very unlikely collision someone is using this key?
I think we use the hatchet__ prefix for things like event id already
There was a problem hiding this comment.
this seems reasonable - I'm actually not sure. My sense is leaving this as just traceparent is fine because we'll need to rename the key anyways, but do you think that it's likely people will ever pass this as a field in the metadata and not intend for it to be used this way?
There was a problem hiding this comment.
Yeah it's highly unlikely... and only breaks things if they use this key AND feature
Easier to review without whitespace because of all the indentation from removing spans
Reworking our OTel setup to instead follow a bunch of other "instrumentor" patterns. See the example for usage - I'll add docs too (modify the existing ones). This will replace our current setup. Tested a bunch with Honeycomb and everything looked great to me 🥳 - this should also let us extend our instrumentation pretty easily without mucking around in the internals of the SDK too much.
I also changed the OTel setup to be an "extra", so you can install the deps with
pip install hatchet-sdk[otel]if you want them, but otherwise don't need to.TODO: