Fix otel context propagation in a few more places missed previsouly#4274
Merged
Fix otel context propagation in a few more places missed previsouly#4274
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4274 +/- ##
=======================================
Coverage 63.06% 63.06%
=======================================
Files 184 184
Lines 19932 19932
Branches 4955 4959 +4
=======================================
Hits 12571 12571
Misses 7358 7358
Partials 3 3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This is a follow-up to #4149 which fixed OTel context propagation in PartialModes and Snapshotter. The Effects.query_move_in_async function has the same pattern: it spawns a task that calls SnapshotQuery.execute_for_shape, but without OTel context propagation the child spans inside would be silently dropped. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Two handlers receive calls that should have OTel context but weren't
setting it before calling ShapeStatus functions that use with_child_span:
1. handle_call({:create_or_wait_shape_handle, shape, otel_ctx}) - the
otel_ctx was passed to opts but not set in the process context before
calling ShapeStatus.fetch_handle_by_shape_critical and add_shape.
2. handle_call({:start_consumer_for_handle, ...}) - previously had a
TODO comment acknowledging the missing context. Now accepts an opts
keyword list (matching get_or_create_shape_handle/3) and ConsumerRegistry
passes the context from within its span.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The OTel context propagation change added an opts argument to ShapeCache.start_consumer_for_handle. Update Repatch stubs in the ConsumerRegistry and ShapeLogCollector tests to accept the new arity.
Move @default_config inside the with_telemetry Sentry.LoggerHandler do branch so it is only defined when actually referenced. Fixes the "module attribute @default_config was set but never used" warning introduced by #4146 (4176bd2), where the attribute lives at the module top level but is only used inside the do branch — the else branch (compiled when Sentry.LoggerHandler is unavailable) never references it.
alco
force-pushed
the
fix-otel-context-effects-move-in
branch
from
e6fb6fa to
2d990f2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #4149 which fixed OTel context propagation in three spawn sites. While auditing the rest of the codebase, three additional places were found where
with_child_spancalls were silently dropping spans due to missing parent context.1.
Effects.query_move_in_async/4(spawned task)The same pattern as the sites fixed in #4149: a
Task.Supervisor.start_childcall wrappingSnapshotQuery.execute_for_shape, but without propagating the OTel context into the spawned task. Fixed by capturing the context before the spawn and attaching it inside the task closure.2.
ShapeCache.handle_call({:create_or_wait_shape_handle, shape, otel_ctx})The
otel_ctxwas already being passed in the GenServer call message (fromElectric.Shapes.get_or_create_shape_handle/3), but the handler was only forwarding it to the Snapshotter — it never set the context on the ShapeCache GenServer process itself. As a result, thewith_child_spancalls insideShapeStatus.fetch_handle_by_shape_criticalandShapeStatus.add_shapewere silently dropped.3.
ShapeCache.handle_call({:start_consumer_for_handle, ...})This handler had a
# TODO: otel ctx from shape log collector?comment acknowledging the gap. The caller,ConsumerRegistry.start_consumer!/2, wraps the GenServer call inOpenTelemetry.with_span(...)but the context wasn't crossing the GenServer call boundary. The public function signature was changed to take anoptskeyword list (matchingget_or_create_shape_handle/3), withConsumerRegistrycapturing the current context inside its span and passing it through.Test plan
mix compilecleanmix test test/electric/telemetry/open_telemetry_test.exs— 8 passingname = shape_status.add_shapeandname = shape_status.fetch_handle_by_shape_criticalrows show up in Honeycomb under shape-creation traces (previously dropped).