getsentry / getsentry/sentry-javascript

cloudflare: Several common bindings are not instrumented

Đang mở
#24,037 1 bình luận 0 reaction 0 người được giao Xem trên GitHub
Cloudflare Workers Feature javascript Spans
Ngôn ngữ chính
TypeScript
Star
8.7k
Fork
1.8k
Merge trung bình
1 ngày 17 giờ
Pull request đã merge (30 ngày)
523

Mô tả

`packages/cloudflare/src/instrumentations/worker/instrumentEnv.ts` detects D1, Queue, R2, RateLimit, Workers AI, DurableObjectNamespace and JSRPC. Nothing else matches, so these fall through untouched:

| Binding | Note |
| -- | -- |
| **KV namespace** (`env.MY_KV`) | the most-used Cloudflare binding. Durable Object storage KV (`ctx.storage.kv`) *is* instrumented, which makes this easy to mistake for covered. |
| Vectorize | `query`, `insert`, `upsert`, `getByIds` |
| Analytics Engine | `writeDataPoint` |
| Pipelines | `send` only, so `isQueue` (which needs `send` + `sendBatch`) misses it |
| Secrets Store | `get` |
| Dispatch namespace | Workers for Platforms |
| Hyperdrive | connection only; the query itself needs the driver instrumented |
| Containers, Browser Rendering | |
| **Cache API** (`caches.default.match` / `.put`) | not a binding, but the same category of missing span. The `cache-client` test suite is about the SDK client cache, not this. |

**Workers KV verification** A live `KvNamespace` binding matches none of the seven duck-type checks, so `instrumentEnv`'s proxy returns it untouched:

```
kvCtor: "KvNamespace", methods: [get, put, delete, list, getWithMetadata]
isJSRPC: false hasIdFromName: false hasSendAndSendBatch: false
hasPrepareBatchExec: false hasHeadPutMultipart: false hasLimit: false
hasRunGatewayToMarkdown: false
```

The same worker, one invocation, with two controls to prove the harness works:

| Call | Span |
| -- | -- |
| `Sentry.startSpan('control-manual-span')` | `test.control \| control-manual-span` |
| `env.MY_R2.put()` (R2 is instrumented, same `env` proxy) | `object.put \| r2_put` |
| `env.MY_R2.head()` | `object.head \| r2_head` |
| `env.MY_KV.put()` | none |
| `env.MY_KV.get()` | none |
| `env.MY_KV.list()` | none |
| `env.MY_KV.delete()` | none |

`KVNamespace` and `getWithMetadata` appear nowhere in any package's `src`. Every `kv` match in `packages/cloudflare/src` is Durable Object storage (`ctx.storage.kv`), including the `durableObjectSqlSpanAllowlist` sibling option at `client.ts:386` that mentions "KV reads/writes".

**One exception:** KV is not completely uncovered across the monorepo. `@sentry/nitro` and `@sentry/nuxt` subscribe to `unstorage` tracing channels (`packages/nitro/src/runtime/hooks/captureStorageEvents.ts:84`) and emit cache spans with `db.system.name` taken from the unstorage driver. A Nitro or Nuxt app on Cloudflare that reads through `useStorage()` over a KV-backed mount therefore does get spans. That path does not help direct `env.MY_KV` access, and does not exist for Next.js, TanStack Start, SvelteKit, Hono, React Router, or a plain Worker. So the gap is real, but scope any new issue to the binding rather than to "KV", and reuse the op naming that instrumentation already established.

**Work item.** Start with the KV binding alone: an `isKVNamespace` duck-type (`get` + `put` + `list` + `getWithMetadata`, and not JSRPC) plus an `instrumentKV` that emits spans matching `instrumentR2`. `getWithMetadata` is the discriminator worth keying on, since `get`/`put`/`delete`/`list` are common enough to risk a false positive. Order the check before `isRateLimit`, which matches anything with a `limit` method. Ship Vectorize and Analytics Engine as follow-ups.

**Prior art ***(tracked)*. Most of this list is already tracked, one issue per binding:

| Binding | Issue |
| -- | -- |
| **Workers KV** | **none** |
| Vectorize | [#20847]() (open) |
| Analytics Engine | [#20860]() (open) |
| Dispatch namespace | [#20859]() (open) |
| Cache API | [#16895]() (open) |
| Hyperdrive / relational DBs | [#16249]() (open) |
| PITR API | [#20831]() (open) |
| Flagship feature flags | [#21184]() (open) |
| `storage.sql` | [#20833]() (open) |
| Pipelines, Secrets Store, Containers, Browser Rendering | none |

Workers KV having no issue was worth double-checking, because two closed issues look like they cover it and do not: [#19384]() "Cloudflare Instrument Async KV Api" and [#20830]() "Cloudflare instrument Sync KV API" are both sub-tickets of [#19106]() (SQLite-backed Durable Object storage). They are about `ctx.storage.kv`, not the `env.MY_KV` binding, and both shipped. So the most-used Cloudflare binding is the one gap with no ticket, and it reads as already done.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Hướng nghiên cứu

Start in packages/cloudflare/src/instrumentations/worker/instrumentEnv.ts and compare the existing R2 detection and instrumentation path. Add KV-specific detection using get, put, list, and getWithMetadata while excluding JSRPC, then instrument the binding with spans matching instrumentR2. Done means direct env.MY_KV operations are instrumented without confusing other bindings, with the relevant Cloudflare instrumentation tests updated or added.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
typescript
Lĩnh vực
backend, observability-sre
Loại issue
Tính năng
Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức độ hoạt động
Sôi nổi
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
72/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.