16 Redis commands. 0 spans. A hook that looked perfectly correct.

This is a submission for DEV's Summer Bug Smash: Clear the Lineup powered by Sentry.

I spent an afternoon reading four lines of Go and agreeing with them. They were wrong.

Not subtly wrong. The function was supposed to attach telemetry to every shard in a Redis ring, and it attached telemetry to none of them. Ring clients ran in production emitting nothing at all, and the code that failed to do it reads like textbook Go.

Here is how it hid, how I found it, and what it looks like in Sentry when you finally put a light on it.

---

Project Overview

What the project does

opentelemetry-go-compile-instrumentation is OpenTelemetry's compile time instrumentation for Go.

You do not add tracing calls to your code. The toolchain rewrites your binary during the build, so the tracing is already inside it when it ships.

Why a bug here is worse than most

That property is the entire appeal, and it is also what makes a defect in it dangerous.

You did not write the instrumentation. You are not reading the instrumentation. You are reading the dashboard it feeds, and you trust it completely, because trusting it is the point.

The project ships hooks for a lot of common libraries. go-redis gets six of them, one per client type. Each is roughly four lines: something constructs a client, the hook runs afterwards, the hook attaches an OpenTelemetry hook to that client.

The one for redis.Ring did nothing at all.

---

Bug Fix or Performance Improvement

The four lines

func afterNewRingClientV9(call hook.HookContext, client *redis.Ring) {
    client.OnNewNode(func(rdb *redis.Client) {
        rdb.AddHook(newOtelRedisHook(rdb.Options().Addr))
    })
}

Read that on its own and it is fine.

A ring is a set of shards. OnNewNode registers a callback that runs for each shard. The callback attaches the telemetry hook. Therefore every shard gets instrumented.

I read it three times and agreed with it three times. Then I went and read NewRing.

What actually happens

OnNewNode only fires for shards created after you register the callback.

NewRing builds a shard for every entry in RingOptions.Addrs during construction, and construction finishes before this hook ever runs.

So by the time afterNewRingClientV9 registers its callback, every shard the caller configured already exists. The callback will never see a single one of them.

If you build a ring the normal way, by listing your Redis addresses in the options, you get zero telemetry. Not degraded. Not partial. Zero. Every command, to every shard, invisible.

The only way to get even one instrumented shard was to construct a ring with no addresses and add them later through SetAddrs. Nobody builds a ring that way.

Why this class of bug is the worst class

There is no error. No warning. No partial data that looks suspicious.

Your Redis spans are simply absent, and absence reads as health.

If you opened a service map and saw no Redis calls, your first thought is not "the instrumentation is broken." It is "I guess Redis is fine." A bug that produces wrong numbers eventually gets caught by somebody who notices the numbers are wrong. A bug that produces no numbers can sit there forever.

Why nobody had reported it

The entire package had no test coverage.

Six client hooks, zero tests. Nothing anywhere was asserting that any of them attached anything.

---

Code

Upstream PR: open-telemetry/opentelemetry-go-compile-instrumentation#1098

The fix

Keep the OnNewNode registration, since it is still correct for shards added later through SetAddrs, and additionally walk the shards that already exist.

 func afterNewRingClientV9(call hook.HookContext, client *redis.Ring) {
    client.OnNewNode(func(rdb *redis.Client) {
        rdb.AddHook(newOtelRedisHook(rdb.Options().Addr))
    })
+   // NewRing builds a shard for every entry in RingOptions.Addrs before this
+   // hook runs, and OnNewNode only fires for shards created after it is
+   // registered. Without this pass the shards a caller configured up front
+   // are never instrumented, which is the common case.
+   attachHookToExistingShards(client)
 }
+
+// attachHookToExistingShards instruments the shards a ring already holds.
+// ForEachShard returns the first error a callback produces; this callback
+// cannot fail, so there is nothing to report.
+func attachHookToExistingShards(client *redis.Ring) {
+   _ = client.ForEachShard(context.Background(), func(_ context.Context, rdb *redis.Client) error {
+       rdb.AddHook(newOtelRedisHook(rdb.Options().Addr))
+       return nil
+   })
+}

Both paths are now covered. Shards present at construction get instrumented by the ForEachShard pass. Shards added later get instrumented by OnNewNode.

On that comment

I left it deliberately, and I would fight to keep it in review.

The next person to read this function will have exactly the reaction I had, which is that OnNewNode looks sufficient. It is not sufficient. The only way to know that is to know when NewRing builds its shards relative to when the hook runs.

That fact belongs in a comment, not in somebody's head.

Testing a Redis hook without a Redis server

This is the part I expected to be hard, because instrumenting a Redis client normally means having a Redis server, and needing a server in CI is how tests end up skipped.

It turns out you do not need one.

Each test builds a client pointed at a port nothing is listening on, then issues a command. The dial fails. But the hook opens its span before handing the command to the next hook in the chain, so a failed connection exercises the instrumentation completely.

Assert that a client span came out carrying the right endpoint and you have tested the thing you actually care about. No server. No container. No fixture.

Test that pins the bug	TestAfterNewRingClientV9_AttachesHookToEachNode
Fails on old code	Yes
Other hooks covered	5
Package coverage	89.2% to 96.9%

---

My Improvements

What changed

Ring clients went from producing no telemetry to producing telemetry for every shard, in the configuration basically everybody uses. Six previously untested hooks now have tests.

Callback registration order is a whole family of bugs

Any API shaped like "register a callback and it will fire for each X" has a question hiding inside it: what about the Xs that already exist?

Some libraries replay for existing items. Some only fire going forward. The method name almost never tells you which.

OnNewNode is arguably honest, the word is right there in the name. But I would bet most people read it as "on each node," because that is what you want it to mean.

Instrumentation needs tests more than normal code, not less

Missing telemetry is harder to notice than wrong telemetry.

Nothing downstream will ever complain on its behalf. A dashboard cannot tell you it is missing a line.

Point it at a dead port

To test a client library hook, aim it somewhere nothing is listening.

The failure path runs through the same instrumentation as the success path, and you skip the entire problem of standing up a real server in CI. I will be reusing this one.

---

Best Use of Sentry

The problem with proving a negative

This bug has an unusual property. The symptom is absent telemetry.

You cannot screenshot a missing span. So the only honest way to prove it is to put both versions in front of a real tracing backend and show the shape of the hole.

I built a reproduction that ships OpenTelemetry spans straight to Sentry over OTLP and runs an identical workload under both attachment strategies:

github.com/vignesh2027/bugsmash-sentry-demo

Three shard ring. Sixteen Redis commands. One cart-checkout transaction wrapping all of them. The only variable in the entire program is which attachment function runs.

Before: the empty waterfall

One bar. 4.49 seconds. Nothing underneath it.

Sixteen Redis commands executed inside that window, across three shards, and the trace shows none of them.

Sit with that image for a second. If somebody handed it to you during an incident, you would conclude this service does not talk to Redis. It talks to Redis sixteen times.

After: the same code, traced

Same code. Same commands. Same 4.5 seconds.

Eleven redis.get and redis.set spans, each one tagged with the shard that served it.

The span math, because the two screenshots do not obviously agree

The fixed run produces 20 Redis spans in total. Only 11 land inside the cart-checkout trace.

The other 9 are redis.ping shard health checks. go-redis runs those on its own goroutines, outside the request context, so Sentry correctly files them as separate root traces.

That detail makes the bug worse than I first understood. The ring's own health monitoring was untraced too, which means a ring quietly losing a shard would not have surfaced either.

Which Sentry tools did the work

Tool	What it did here
Distributed Tracing	Ingested the real OTel spans over OTLP, so I am watching the actual instrumentation, not a reimplementation of it
Error Monitoring	The failed commands are captured on a scope tagged with operation, key, and hook mode, so the Issue says what happened
Trace linked errors	sentryotel.NewOtelIntegration() attaches captures to the active span, so the error and its span land together
Span attributes	Every span carries redis.shard, which proves every shard is covered, not merely that something fired

That last row matters more than it sounds. A partial fix that instrumented one shard out of three would look perfectly healthy on a span count, and would still be broken. Shard level attributes are what turn "some spans exist" into "coverage is complete."

Two things that cost me time

The OTLP endpoint has a segment you will miss.

DSN    https://<key>@o<org>.ingest.<region>.sentry.io/<project>
OTLP   https://o<org>.ingest.<region>.sentry.io/api/<project>/integration/otlp/v1/traces
auth   x-sentry-auth: sentry sentry_key=<key>

Leave out /integration/ and you get 404 Not Found with an empty body, which tells you nothing about which half of the URL was wrong.

sentry-go v0.48 removed NewSentrySpanProcessor.

The tracer provider pattern most tutorials still show will not compile. OTLP export is the current path, and the otel package is now reduced to linking errors onto the trace. Also worth knowing: OTLP ingestion is in open beta and span events are dropped during ingest, which is why the diagnostic detail here lives in attributes. That turned out to be the better design regardless.

The evidence travels with the code

I added a local span counter as a span processor, so the demo prints its own tally. You do not have to take my word for what the Sentry UI showed:

BUGGY                          FIXED
ran:    16 commands            ran:    16 commands
redis spans observed: 0        redis spans observed: 20

Clone it, point it at your own DSN, and you will get the same two numbers.