Support for OpenTelemetry's db.query.text field#360
Draft
Conversation
JoshMock
force-pushed
the
otel-db.query.text
branch
from
1118ebc to
9ee3ec1
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds opt-in recording of OpenTelemetry’s db.query.text attribute for Elasticsearch “search-like” requests (with best-effort sanitization and truncation), and refactors OpenTelemetry instrumentation into a first-class transport middleware. It also commits Spec Kit artifacts for the spec-driven development experiment.
Changes:
- Introduce
OpenTelemetryMiddleware(plus middleware-enginewrapsupport) and wire it intoTransport.request(). - Add sanitization helpers (
sanitizeJsonBody,sanitizeStringQuery,sanitizeNdjsonBody) and extensive unit tests for sanitization + OTel behavior. - Update packaging/exports and commit Spec Kit specification artifacts under
specs/and.specify/.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/transport.test.ts | Removes OTel tests from transport unit suite (moved to dedicated middleware test). |
| test/unit/security.test.ts | Adds unit tests for new sanitization helpers. |
| test/unit/middleware/opentelemetry.test.ts | Adds comprehensive OTel middleware tests, including db.query.text behavior, truncation, and env-var controls. |
| src/symbols.ts | Removes OTel-related internal symbols in favor of middleware approach. |
| src/security.ts | Adds sanitization helpers used for db.query.text capture. |
| src/middleware/types.ts | Adds OPEN_TELEMETRY middleware name/priority and introduces wrap middleware hook. |
| src/middleware/index.ts | Exposes the new OpenTelemetry middleware and related constants/types. |
| src/middleware/OpenTelemetry.ts | Implements OTel span creation, attribute setting, and db.query.text capture/sanitization/truncation. |
| src/middleware/MiddlewareEngine.ts | Adds executeWrap() to compose/execute wrap-style middleware. |
| src/Transport.ts | Refactors request flow to run through middleware wrapping; wires OTel middleware; exports OpenTelemetryOptions from middleware module. |
| specs/002-capture-search-query-default-on/tasks.md | Spec Kit task breakdown for the feature. |
| specs/002-capture-search-query-default-on/spec.md | Spec for env-var disable, truncation, and per-request overrides. |
| specs/002-capture-search-query-default-on/research.md | Research notes and decisions for sanitization strategy and precedence rules. |
| specs/002-capture-search-query-default-on/plan.md | Implementation plan for the combined 001+002 feature scope. |
| specs/002-capture-search-query-default-on/data-model.md | Data model for captureSearchQuery and endpoint categorization. |
| specs/002-capture-search-query-default-on/contracts/otel-api.md | Public contract for OpenTelemetryOptions and security module exports. |
| specs/002-capture-search-query-default-on/checklists/requirements.md | Requirements checklist for the 002 spec. |
| specs/001-otel-search-query-capture/spec.md | Initial 001 feature spec for db.query.text capture. |
| specs/001-otel-search-query-capture/checklists/requirements.md | Requirements checklist for the 001 spec. |
| package.json | Updates exports mappings (subpath import/require resolution). |
| .specify/templates/tasks-template.md | Adds tasks template for Spec Kit workflow. |
| .specify/templates/spec-template.md | Adds spec template for Spec Kit workflow. |
| .specify/templates/plan-template.md | Adds plan template for Spec Kit workflow. |
| .specify/templates/constitution-template.md | Adds constitution template for Spec Kit workflow. |
| .specify/templates/checklist-template.md | Adds checklist template for Spec Kit workflow. |
| .specify/templates/agent-file-template.md | Adds agent context template for Spec Kit workflow. |
| .specify/scripts/bash/update-agent-context.sh | Adds script to update agent context files based on plans. |
| .specify/scripts/bash/setup-plan.sh | Adds script to initialize plan files from templates. |
| .specify/scripts/bash/create-new-feature.sh | Adds script to create new Spec Kit feature directories/branches. |
| .specify/scripts/bash/common.sh | Adds shared helper functions for Spec Kit scripts. |
| .specify/scripts/bash/check-prerequisites.sh | Adds consolidated prerequisite checker for Spec Kit workflow. |
| .specify/memory/constitution.md | Adds/ratifies the project constitution for spec-driven workflow. |
| .npmignore | Excludes Spec Kit artifacts and additional repo files from npm package. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+159
to
+170
| if ((otelOptions.captureSearchQuery ?? false) && params.meta?.name != null && SEARCH_LIKE_ENDPOINTS.has(params.meta.name)) { | ||
| const rawBody = NDJSON_ENDPOINTS.has(params.meta.name) ? params.bulkBody : params.body | ||
| if (rawBody != null && rawBody !== '' && !isStream(rawBody)) { | ||
| const bodyStr = typeof rawBody === 'string' ? rawBody : JSON.stringify(rawBody) | ||
| let sanitized: string | null | ||
| if (NDJSON_ENDPOINTS.has(params.meta.name)) { | ||
| sanitized = sanitizeNdjsonBody(bodyStr) | ||
| } else if (STRING_QUERY_ENDPOINTS.has(params.meta.name)) { | ||
| sanitized = sanitizeStringQuery(bodyStr) | ||
| } else { | ||
| sanitized = sanitizeJsonBody(bodyStr) | ||
| } |
Comment on lines
+160
to
+174
| const rawBody = NDJSON_ENDPOINTS.has(params.meta.name) ? params.bulkBody : params.body | ||
| if (rawBody != null && rawBody !== '' && !isStream(rawBody)) { | ||
| const bodyStr = typeof rawBody === 'string' ? rawBody : JSON.stringify(rawBody) | ||
| let sanitized: string | null | ||
| if (NDJSON_ENDPOINTS.has(params.meta.name)) { | ||
| sanitized = sanitizeNdjsonBody(bodyStr) | ||
| } else if (STRING_QUERY_ENDPOINTS.has(params.meta.name)) { | ||
| sanitized = sanitizeStringQuery(bodyStr) | ||
| } else { | ||
| sanitized = sanitizeJsonBody(bodyStr) | ||
| } | ||
| if (sanitized !== null) { | ||
| attributes['db.query.text'] = sanitized.slice(0, SEARCH_QUERY_MAX_LENGTH) | ||
| } | ||
| } |
Comment on lines
+72
to
+83
| wrap = async (params: TransportRequestParams, options: TransportRequestOptions, next: () => Promise<any>): Promise<any> => { | ||
| const otelOptions: OpenTelemetryOptions = Object.assign({}, this.transportOptions, options.openTelemetry ?? {}) | ||
|
|
||
| if (!(otelOptions.enabled ?? true) || params.meta?.name == null) { | ||
| return await next() | ||
| } | ||
|
|
||
| let context = opentelemetry.context.active() | ||
| if (otelOptions.suppressInternalInstrumentation ?? false) { | ||
| context = suppressTracing(context) | ||
| } | ||
|
|
Comment on lines
+44
to
+50
| try { | ||
| if (mw.wrap == null) return await next() | ||
| return await mw.wrap(params, options, next) | ||
| } catch (error) { | ||
| if (error instanceof ElasticsearchClientError) throw error | ||
| throw new MiddlewareException(`Middleware ${mw.name} failed in wrap`, { cause: error }) | ||
| } |
Comment on lines
+717
to
+719
| params, options, async () => await this._request(params, { ...options, meta: true }) | ||
| ) as TransportResult | ||
| return (options as TransportRequestOptionsWithMeta).meta ? result : result.body |
Comment on lines
15
to
24
| "./lib/*": { | ||
| "import": "./esm/*.js", | ||
| "require": "./lib/*.js", | ||
| "types": "./lib/*.d.ts" | ||
| }, | ||
| "./*.js": { | ||
| "import": "./esm/*.js", | ||
| "import": "./esm/*", | ||
| "require": "./lib/*.js", | ||
| "types": "./lib/*.d.ts" | ||
| }, | ||
| "./*": { | ||
| "import": "./esm/*.js", | ||
| "require": "./lib/*.js", | ||
| "types": "./lib/*.d.ts" | ||
| "import": "./esm/*", | ||
| "require": "./*", | ||
| "types": "./*.d.ts" | ||
| } |
Comment on lines
+86
to
+98
| return await this.tracer.startActiveSpan(params.meta.name, { attributes, kind: SpanKind.CLIENT }, context, async (span) => { | ||
| try { | ||
| const result = await next() | ||
| this.setResponseAttributes(span, result) | ||
| return result | ||
| } catch (err: any) { | ||
| span.recordException(err as Exception) | ||
| span.setStatus({ code: SpanStatusCode.ERROR }) | ||
| span.setAttribute('error.type', err.name ?? 'Error') | ||
| throw err | ||
| } finally { | ||
| span.end() | ||
| } |
JoshMock
force-pushed
the
otel-db.query.text
branch
from
9ee3ec1 to
90660aa
Compare
JoshMock
force-pushed
the
otel-db.query.text
branch
from
90660aa to
372d539
Compare
Collaborator
💚 Build Succeeded
Performance benchmark comparisonBenchmark details (Ubuntu)Performance BenchmarksTransport#constructor - UndiciConnectionWeightedConnectionPool
ClusterConnectionPool
CloudConnectionPool
Transport#constructor - HttpConnectionWeightedConnectionPool
ClusterConnectionPool
CloudConnectionPool
GC BenchmarksSearch request: defaults
Search request: defaults + compression
This comment was automatically generated by the comparative benchmark workflow. History
|
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
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.
Records a
db.query.textfield to OpenTelemetry spans for search-like queries sent to Elasticsearch, adding support for an optional semantic convention noted in the OTel docs. Best-effort sanitization is performed, with JSON DSL queries having all primitive values substituted with?. Parameterized string queries have all parameters dropped. Any other string-based queries are NOT recorded, as sanitization is less straightforward.This also refactors OpenTelemetry into a middleware module, which is an ongoing effort we are applying to the transport library. See elastic/elasticsearch-js#2902
NOTE: This is a spec-driven development experiment, using Spec Kit. All the specification artifacts in
.specify/andspecs/will be committed for now, at least until the experiment ends.