diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 000000000..0a0145661 --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,73 @@ +# Code of Conduct + +## Our Pledge + +We as members, contributors, and leaders pledge to make participation in our community a harassment-free experience for everyone, regardless of age, body size, visible or invisible disability, ethnicity, sex characteristics, gender identity and expression, level of experience, education, socio-economic status, nationality, personal appearance, race, caste, color, religion, or sexual identity and orientation. + +We pledge to act and interact in ways that contribute to an open, welcoming, diverse, inclusive, and healthy community. + +## Our Standards + +Examples of behavior that contributes to a positive environment include: + +- Demonstrating empathy and kindness toward other people +- Being respectful of differing opinions, viewpoints, and experiences +- Giving and gracefully accepting constructive feedback +- Accepting responsibility and apologizing to those affected by our mistakes, and learning from the experience +- Focusing on what is best not just for us as individuals, but for the overall community + +Examples of unacceptable behavior include: + +- The use of sexualized language or imagery, and sexual attention or advances of any kind +- Trolling, insulting or derogatory comments, and personal or political attacks +- Public or private harassment +- Publishing others' private information, such as a physical or email address, without their explicit permission +- Other conduct which could reasonably be considered inappropriate in a professional setting + +## Enforcement Responsibilities + +Community leaders are responsible for clarifying and enforcing our standards of acceptable behavior and will take appropriate and fair corrective action in response to any behavior that they deem inappropriate, threatening, offensive, or harmful. + +Community leaders have the right and responsibility to remove, edit, or reject comments, commits, code, wiki edits, issues, and other contributions that are not aligned to this Code of Conduct, and will communicate reasons for moderation decisions when appropriate. + +## Scope + +This Code of Conduct applies within all community spaces, and also applies when an individual is officially representing the community in public spaces. Examples include using an official email address, posting via an official social media account, or acting as an appointed representative at an online or offline event. + +## Enforcement + +Instances of abusive, harassing, or otherwise unacceptable behavior may be reported to the community leaders responsible for enforcement at conduct@clickhouse.com. All complaints will be reviewed and investigated promptly and fairly. + +All community leaders are obligated to respect the privacy and security of the reporter of any incident. + +## Enforcement Guidelines + +Community leaders will follow these guidelines in determining the consequences for any action they deem in violation of this Code of Conduct: + +### 1. Correction + +**Community Impact**: Use of inappropriate language or other behavior deemed unprofessional or unwelcome in the community. + +**Consequence**: A private, written warning from community leaders, providing clarity around the nature of the violation and an explanation of why the behavior was inappropriate. A public apology may be requested. + +### 2. Warning + +**Community Impact**: A violation through a single incident or series of actions. + +**Consequence**: A warning with consequences for continued behavior. No interaction with the people involved, including unsolicited interaction with those enforcing the Code of Conduct, for a specified period of time. This includes avoiding interactions in community spaces as well as external channels like social media. Violating these terms may lead to a temporary or permanent ban. + +### 3. Temporary Ban + +**Community Impact**: A serious violation of community standards, including sustained inappropriate behavior. + +**Consequence**: A temporary ban from any sort of interaction or public communication with the community for a specified period of time. No public or private interaction with the people involved, including unsolicited interaction with those enforcing the Code of Conduct, is allowed during this period. Violating these terms may lead to a permanent ban. + +### 4. Permanent Ban + +**Community Impact**: Demonstrating a pattern of violation of community standards, including sustained inappropriate behavior, harassment of an individual, or aggression toward or disparagement of classes of individuals. + +**Consequence**: A permanent ban from any sort of public interaction within the community. + +## Attribution + +This Code of Conduct is adapted from the [Contributor Covenant](https://www.contributor-covenant.org), version 2.1, available at https://www.contributor-covenant.org/version/2/1/code_of_conduct.html. diff --git a/docs/howto/tracing.md b/docs/howto/tracing.md index d8e3853e6..233b27dde 100644 --- a/docs/howto/tracing.md +++ b/docs/howto/tracing.md @@ -114,7 +114,10 @@ before talking to the server), the client invokes `db.operation.name`, `db.collection.name` and `clickhouse.request.sent_rows` (insert; the row count is recorded for array-based inserts only), `clickhouse.request.query_id`, and - `clickhouse.request.session_id`. + `clickhouse.request.session_id`. Any per-request + [`span_attributes`](#enriching-spans-with-span_attributes) and, when + [`dangerously_log_query_text`](#logging-the-raw-query-text) is enabled, the + raw SQL as `db.query.text` are merged into this initial bag too. 2. Inside `fn`, the network operation runs with the span as the active span (when the context manager supports it; see above). 3. `span.setAttributes({ 'clickhouse.request.query_id': })` - @@ -122,7 +125,14 @@ before talking to the server), the client invokes one and the connection layer generated it. Once the response arrives, the span also gets `db.response.status_code` (HTTP status) and, when the `X-ClickHouse-Summary` header is present (e.g. with `wait_end_of_query`), - `clickhouse.summary.*` counters (`read_rows`, `written_rows`, …). + the `clickhouse.summary.*` counters. **Every** key present in the parsed + summary is recorded (the set is not hardcoded), so you get `read_rows`, + `read_bytes`, `written_rows`, `written_bytes`, `result_rows`, + `result_bytes`, `total_rows_to_read`, `elapsed_ns`, and — on servers that + report them — `memory_usage` (peak query memory, in bytes), + `real_time_microseconds`, and any future server-side additions for free. + These counters are attached to every operation span, including the outer + `clickhouse.query` span. 4. On success, the span status is left **unset**, per the OTEL span status spec for client spans. On failure, `span.setAttributes({ 'error.type': })` (plus @@ -150,8 +160,11 @@ propagates to the caller of `query` / `command` / `exec` / `insert` / > ends when the result set is fully consumed (`text()`/`json()` resolve, or > the `stream()` is read to completion), closed via `close()`, or fails > (the error is recorded on this span). When it ends it carries the final -> `clickhouse.response.decoded_bytes` and, for row-streaming consumption, -> `db.response.returned_rows` metrics. +> `clickhouse.response.decoded_bytes` and `db.response.returned_rows` +> metrics. `returned_rows` is recorded both for row-streaming consumption +> (`stream()`, and `json()` on the streamable JSON formats) and for +> non-streaming `json()` on `JSON` / `JSONObjectEachRow` / the other +> single-document JSON formats. > > This split makes it easy to distinguish the original request round-trip from > a stream that may never end (e.g. tailing a live materialized view). If the @@ -159,6 +172,61 @@ propagates to the caller of `query` / `command` / `exec` / `insert` / > is never ended. For `command`/`exec`/`insert`/`ping`, a single span ends > when the method returns. +## Enriching spans with `span_attributes` + +Every request method (`query` / `command` / `exec` / `insert` / `ping`) +accepts an optional `span_attributes` bag that is merged into the operation +span. This is the recommended way to attach application-level context to your +traces — for example, mirroring the tags you also send to ClickHouse via the +[`log_comment`](https://clickhouse.com/docs/operations/settings/settings#log_comment) +setting so the same context is visible both in `system.query_log` and in your +tracing backend: + +```ts +const tag = { + route: "events.getAgentGraphData", + tenant: "acme", + surface: "api", +}; + +await client.query({ + query: "SELECT * FROM events WHERE tenant = {tenant:String}", + query_params: { tenant: tag.tenant }, + // Visible in ClickHouse's system.query_log + clickhouse_settings: { log_comment: JSON.stringify(tag) }, + // Visible on the tracing span + span_attributes: { + "app.route": tag.route, + "app.tenant": tag.tenant, + "app.surface": tag.surface, + }, +}); +``` + +Values may be `string`, `number`, or `boolean`. Caller-provided attributes +**never override** the client's own semantic-convention attributes (`db.*`, +`server.*`, `clickhouse.*`) on a key collision. `span_attributes` are ignored +when no tracer is configured. + +## Logging the raw query text + +By default the client **never** attaches the raw SQL to spans or logs, because +a statement can contain sensitive data inlined as literals. Set +`dangerously_log_query_text: true` at client creation to opt in: + +```ts +const client = createClient({ + tracer: trace.getTracer("clickhouse-js"), + dangerously_log_query_text: true, +}); +``` + +When enabled, the raw SQL is attached to every operation span as the OTEL +[`db.query.text`](https://opentelemetry.io/docs/specs/semconv/database/database-spans/#common-attributes) +attribute, and (Node.js) included in the `error`-level log emitted when a +request fails. Bound `query_params` values and credentials are **never** logged +or traced, regardless of this setting. + ## Adapter recipes: `requireParentSpan` and suppressing nested HTTP spans OpenTelemetry auto-instrumentation packages commonly expose two options that diff --git a/package-lock.json b/package-lock.json index 9207abdf2..20da6e23c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7708,7 +7708,7 @@ }, "packages/client-node": { "name": "@clickhouse/client", - "version": "1.23.1", + "version": "1.24.0", "license": "Apache-2.0", "devDependencies": { "simdjson": "^0.9.2" @@ -7719,7 +7719,7 @@ }, "packages/client-web": { "name": "@clickhouse/client-web", - "version": "1.23.1", + "version": "1.24.0", "license": "Apache-2.0" }, "packages/datatype-parser": { diff --git a/packages/client-common/__tests__/integration/select_query_binding.test.ts b/packages/client-common/__tests__/integration/select_query_binding.test.ts index 703536b79..b8aff351c 100644 --- a/packages/client-common/__tests__/integration/select_query_binding.test.ts +++ b/packages/client-common/__tests__/integration/select_query_binding.test.ts @@ -268,6 +268,35 @@ describe("select with query binding", () => { const response = await rs.text(); expect(response).toBe('"2022-05-02 13:25:55.123456789"\n'); }); + + it("handles Array(Date) in a parameterized query", async () => { + const rs = await client.query({ + query: "SELECT {dates: Array(Date)} AS dates", + format: "JSONEachRow", + query_params: { + dates: [ + new Date(Date.UTC(2023, 4, 5)), + new Date(Date.UTC(2021, 0, 2)), + ], + }, + }); + + expect(await rs.json()).toEqual([ + { dates: ["2023-05-05", "2021-01-02"] }, + ]); + }); + + it("binds a Date inside Array(DateTime) at day precision (time is dropped)", async () => { + const rs = await client.query({ + query: "SELECT {dates: Array(DateTime)} AS dates", + format: "JSONEachRow", + query_params: { + dates: [new Date(Date.UTC(2022, 4, 2, 13, 25, 55))], + }, + }); + + expect(await rs.json()).toEqual([{ dates: ["2022-05-02 00:00:00"] }]); + }); }); it("handles an array of strings in a parameterized query", async () => { diff --git a/packages/client-common/__tests__/unit/format_query_params.test.ts b/packages/client-common/__tests__/unit/format_query_params.test.ts index 1da0e6ffc..67d45205b 100644 --- a/packages/client-common/__tests__/unit/format_query_params.test.ts +++ b/packages/client-common/__tests__/unit/format_query_params.test.ts @@ -249,4 +249,52 @@ describe("formatQueryParams", () => { }), ).toBe("{'name':'test','flags':[TRUE,FALSE],'tuple':(FALSE,TRUE)}"); }); + + it("formats a Date inside an array as a quoted date string", () => { + expect( + formatQueryParams({ + value: [new Date(Date.UTC(2022, 6, 29, 7, 52, 14))], + }), + ).toBe("['2022-07-29']"); + }); + + it("formats a Date inside a nested array as a quoted date string", () => { + expect( + formatQueryParams({ + value: [[new Date(Date.UTC(2023, 4, 5))]], + }), + ).toBe("[['2023-05-05']]"); + }); + + it("formats a Date inside a tuple as a quoted date string", () => { + expect( + formatQueryParams({ + value: new TupleParam([new Date(Date.UTC(2023, 4, 5))]), + }), + ).toBe("('2023-05-05')"); + }); + + it("formats a Date inside an object value as a quoted date string", () => { + expect( + formatQueryParams({ + value: { d: new Date(Date.UTC(2023, 4, 5)) }, + }), + ).toBe("{'d':'2023-05-05'}"); + }); + + it("uses the UTC date and drops the time for a Date inside an array", () => { + expect( + formatQueryParams({ + value: [new Date(Date.UTC(2022, 6, 29, 23, 59, 59, 999))], + }), + ).toBe("['2022-07-29']"); + }); + + it("formats a Date alongside other types inside an array", () => { + expect( + formatQueryParams({ + value: [new Date(Date.UTC(2023, 4, 5)), "foo", 42, null], + }), + ).toBe("['2023-05-05','foo',42,NULL]"); + }); }); diff --git a/packages/client-common/__tests__/unit/tracing.test.ts b/packages/client-common/__tests__/unit/tracing.test.ts index 1980b77f0..f77d86bc2 100644 --- a/packages/client-common/__tests__/unit/tracing.test.ts +++ b/packages/client-common/__tests__/unit/tracing.test.ts @@ -98,6 +98,7 @@ function makePing(impl?: () => Promise) { function buildClient( tracer: ClickHouseTracer | undefined, overrides: Partial = {}, + configOverrides: Record = {}, ): ClickHouseClient { const connection: MockConnection = { query: makeQuery(overrides.query), @@ -112,6 +113,7 @@ function buildClient( database: "my_db", application: "my_app", tracer, + ...configOverrides, impl: { make_connection: () => connection as any, make_result_set: ((_s, _f, q, _log, _h, _j, span) => ({ @@ -337,6 +339,8 @@ describe("tracer", () => { result_rows: "5", result_bytes: "50", elapsed_ns: "1000", + memory_usage: "4580679", + real_time_microseconds: "12345", }, }), }); @@ -344,9 +348,176 @@ describe("tracer", () => { const attrs = spans[0].attributes; expect(attrs["db.response.status_code"]).toBe(200); expect(attrs["clickhouse.summary.read_rows"]).toBe("10"); + expect(attrs["clickhouse.summary.read_bytes"]).toBe("100"); expect(attrs["clickhouse.summary.written_rows"]).toBe("5"); + expect(attrs["clickhouse.summary.written_bytes"]).toBe("50"); + expect(attrs["clickhouse.summary.result_rows"]).toBe("5"); expect(attrs["clickhouse.summary.result_bytes"]).toBe("50"); + expect(attrs["clickhouse.summary.total_rows_to_read"]).toBe("10"); expect(attrs["clickhouse.summary.elapsed_ns"]).toBe("1000"); + expect(attrs["clickhouse.summary.memory_usage"]).toBe("4580679"); + expect(attrs["clickhouse.summary.real_time_microseconds"]).toBe("12345"); + }); + + it("records clickhouse.summary.* attributes on the query span when the summary is present", async () => { + const { tracer, spans } = createRecordingTracer(); + const client = buildClient(tracer, { + query: async () => ({ + stream: {} as any, + query_id: "q-1", + response_headers: {}, + http_status_code: 200, + summary: { + read_rows: "6568", + read_bytes: "3894304", + written_rows: "0", + written_bytes: "0", + total_rows_to_read: "6568", + result_rows: "0", + result_bytes: "0", + elapsed_ns: "10693523", + memory_usage: "4580679", + }, + }), + }); + await client.query({ query: "SELECT 1" }); + // The summary is attached to the outer clickhouse.query span. + const attrs = spans[0].attributes; + expect(spans[0].name).toBe(ClickHouseSpanNames.query); + expect(attrs["clickhouse.summary.read_rows"]).toBe("6568"); + expect(attrs["clickhouse.summary.total_rows_to_read"]).toBe("6568"); + expect(attrs["clickhouse.summary.elapsed_ns"]).toBe("10693523"); + expect(attrs["clickhouse.summary.memory_usage"]).toBe("4580679"); + }); + + it("omits optional summary attributes that the server did not return", async () => { + const { tracer, spans } = createRecordingTracer(); + const client = buildClient(tracer, { + command: async () => ({ + query_id: "c-1", + response_headers: {}, + summary: { + read_rows: "10", + read_bytes: "100", + written_rows: "5", + written_bytes: "50", + total_rows_to_read: "10", + result_rows: "5", + result_bytes: "50", + elapsed_ns: "1000", + // memory_usage / real_time_microseconds absent (older server). + }, + }), + }); + await client.command({ query: "INSERT INTO t SELECT * FROM s" }); + const attrs = spans[0].attributes; + expect("clickhouse.summary.memory_usage" in attrs).toBe(false); + expect("clickhouse.summary.real_time_microseconds" in attrs).toBe(false); + }); + + it("does NOT attach db.query.text by default", async () => { + const { tracer, spans } = createRecordingTracer(); + const client = buildClient(tracer); + await client.query({ query: "SELECT secret_column FROM secrets" }); + expect("db.query.text" in spans[0].initialAttributes).toBe(false); + }); + + it("attaches db.query.text when dangerously_log_query_text is enabled", async () => { + const { tracer, spans } = createRecordingTracer(); + const client = buildClient( + tracer, + {}, + { dangerously_log_query_text: true }, + ); + await client.query({ query: "SELECT 42" }); + // The query span carries the actual SQL sent (with the FORMAT suffix). + expect(spans[0].initialAttributes["db.query.text"]).toBe( + "SELECT 42 \nFORMAT JSON", + ); + }); + + it("attaches db.query.text for command/exec/insert when enabled", async () => { + const { tracer, spans } = createRecordingTracer(); + const client = buildClient( + tracer, + {}, + { dangerously_log_query_text: true }, + ); + await client.command({ query: "CREATE TABLE t (a UInt8) ENGINE = Memory" }); + await client.exec({ query: "SELECT 1 FORMAT CSV" }); + await client.insert({ table: "my_table", values: [{ a: 1 }] }); + const byName = (name: string) => spans.find((s) => s.name === name)!; + expect( + byName(ClickHouseSpanNames.command).initialAttributes["db.query.text"], + ).toBe("CREATE TABLE t (a UInt8) ENGINE = Memory"); + expect( + byName(ClickHouseSpanNames.exec).initialAttributes["db.query.text"], + ).toBe("SELECT 1 FORMAT CSV"); + expect( + byName(ClickHouseSpanNames.insert).initialAttributes["db.query.text"], + ).toContain("INSERT INTO my_table"); + }); + + it("merges caller-provided span_attributes onto the span", async () => { + const { tracer, spans } = createRecordingTracer(); + const client = buildClient(tracer); + await client.query({ + query: "SELECT 1", + span_attributes: { + "tag.route": "events.getAgentGraphData", + "tag.projectId": "cmbam0px50001ad08fr8ls6ok", + "tag.tag_schema_version": 1, + }, + }); + const attrs = spans[0].initialAttributes; + expect(attrs["tag.route"]).toBe("events.getAgentGraphData"); + expect(attrs["tag.projectId"]).toBe("cmbam0px50001ad08fr8ls6ok"); + expect(attrs["tag.tag_schema_version"]).toBe(1); + // Core semantic-convention attributes are still present. + expect(attrs["db.system.name"]).toBe("clickhouse"); + }); + + it("ignores db.query.text in span_attributes when the flag is disabled (no leak)", async () => { + const { tracer, spans } = createRecordingTracer(); + const client = buildClient(tracer); + await client.query({ + query: "SELECT 1", + span_attributes: { "db.query.text": "SELECT secret FROM secrets" }, + }); + // db.query.text must never be settable via span_attributes. + expect("db.query.text" in spans[0].initialAttributes).toBe(false); + }); + + it("does not let span_attributes override db.query.text when the flag is enabled", async () => { + const { tracer, spans } = createRecordingTracer(); + const client = buildClient( + tracer, + {}, + { dangerously_log_query_text: true }, + ); + await client.query({ + query: "SELECT 1", + span_attributes: { "db.query.text": "SELECT secret FROM secrets" }, + }); + // Only the client-controlled query text is recorded, not the caller's. + expect(spans[0].initialAttributes["db.query.text"]).toBe( + "SELECT 1 \nFORMAT JSON", + ); + }); + + it("does not let span_attributes override core attributes", async () => { + const { tracer, spans } = createRecordingTracer(); + const client = buildClient(tracer); + await client.query({ + query: "SELECT 1", + span_attributes: { + "db.system.name": "not-clickhouse", + "db.namespace": "hijacked", + }, + }); + const attrs = spans[0].initialAttributes; + expect(attrs["db.system.name"]).toBe("clickhouse"); + expect(attrs["db.namespace"]).toBe("my_db"); }); it("emits a span for ping()", async () => { diff --git a/packages/client-common/src/clickhouse_types.ts b/packages/client-common/src/clickhouse_types.ts index f8cc8d936..c0ff4cdba 100644 --- a/packages/client-common/src/clickhouse_types.ts +++ b/packages/client-common/src/clickhouse_types.ts @@ -27,6 +27,8 @@ export interface ClickHouseSummary { result_rows: string; result_bytes: string; elapsed_ns: string; + /** Peak memory usage of the query, in bytes. */ + memory_usage?: string; /** Available only after ClickHouse 24.9 */ real_time_microseconds?: string; } diff --git a/packages/client-common/src/client.ts b/packages/client-common/src/client.ts index e10072f15..a97955013 100644 --- a/packages/client-common/src/client.ts +++ b/packages/client-common/src/client.ts @@ -37,6 +37,17 @@ import { export interface BaseQueryParams { /** ClickHouse's settings that can be applied on query level. */ clickhouse_settings?: ClickHouseSettings; + /** + * Extra attributes to attach to the tracing span emitted for this request + * (when a {@link BaseClickHouseClientConfigOptions.tracer} is configured). + * Useful for enriching spans with application-level context - e.g. mirroring + * the values you also send via the ClickHouse `log_comment` setting (route, + * tenant, surface, etc.) so that they are visible in your traces. + * + * These attributes never override the client's own semantic-convention + * attributes (`db.*`, `server.*`, `clickhouse.*`) on key collisions. + * Ignored when no tracer is configured. */ + span_attributes?: Record; /** Parameters for query binding. https://clickhouse.com/docs/en/interfaces/http/#cli-queries-with-parameters */ query_params?: Record; /** AbortSignal instance to cancel a request in progress. */ @@ -186,7 +197,7 @@ export interface InsertParams< * This is the default behavior for the Node.js version. */ export type PingParamsWithEndpoint = { select: false } & Pick< BaseQueryParams, - "abort_signal" | "http_headers" + "abort_signal" | "http_headers" | "span_attributes" >; /** Parameters for the health-check request - using a SELECT query. * This is the default behavior for the Web version, as the `/ping` endpoint does not support CORS. @@ -271,11 +282,15 @@ export class ClickHouseClient { ClickHouseSpanNames.query, { kind: ClickHouseSpanKind.CLIENT, - attributes: this.withBaseSpanAttributes({ - "clickhouse.response.format": format, - "clickhouse.request.query_id": queryParams.query_id, - "clickhouse.request.session_id": queryParams.session_id, - }), + attributes: this.withBaseSpanAttributes( + { + "clickhouse.response.format": format, + "clickhouse.request.query_id": queryParams.query_id, + "clickhouse.request.session_id": queryParams.session_id, + }, + params, + query, + ), }, async (span) => { let queryResult; @@ -289,11 +304,16 @@ export class ClickHouseClient { span.end(); throw err; } - const { stream, query_id, response_headers, http_status_code } = - queryResult; + const { + stream, + query_id, + summary, + response_headers, + http_status_code, + } = queryResult; // The query_id may have been generated by the connection layer. span.setAttributes({ "clickhouse.request.query_id": query_id }); - setResponseSpanAttributes(span, { http_status_code }); + setResponseSpanAttributes(span, { http_status_code, summary }); // The clickhouse.query span covers the HTTP request lifetime only: // it ends here, once response headers are received. A separate // clickhouse.query.stream child span is started and handed to the @@ -358,10 +378,14 @@ export class ClickHouseClient { ClickHouseSpanNames.command, { kind: ClickHouseSpanKind.CLIENT, - attributes: this.withBaseSpanAttributes({ - "clickhouse.request.query_id": queryParams.query_id, - "clickhouse.request.session_id": queryParams.session_id, - }), + attributes: this.withBaseSpanAttributes( + { + "clickhouse.request.query_id": queryParams.query_id, + "clickhouse.request.session_id": queryParams.session_id, + }, + params, + query, + ), }, async (span) => { try { @@ -406,10 +430,14 @@ export class ClickHouseClient { ClickHouseSpanNames.exec, { kind: ClickHouseSpanKind.CLIENT, - attributes: this.withBaseSpanAttributes({ - "clickhouse.request.query_id": queryParams.query_id, - "clickhouse.request.session_id": queryParams.session_id, - }), + attributes: this.withBaseSpanAttributes( + { + "clickhouse.request.query_id": queryParams.query_id, + "clickhouse.request.session_id": queryParams.session_id, + }, + params, + query, + ), }, async (span) => { try { @@ -458,18 +486,22 @@ export class ClickHouseClient { ClickHouseSpanNames.insert, { kind: ClickHouseSpanKind.CLIENT, - attributes: this.withBaseSpanAttributes({ - "db.operation.name": "INSERT", - "db.collection.name": params.table, - "clickhouse.request.format": format, - "clickhouse.request.query_id": queryParams.query_id, - "clickhouse.request.session_id": queryParams.session_id, - // Only known up front for array-based inserts; for streamed - // inserts, the row count is not observable by the client. - "clickhouse.request.sent_rows": Array.isArray(params.values) - ? params.values.length - : undefined, - }), + attributes: this.withBaseSpanAttributes( + { + "db.operation.name": "INSERT", + "db.collection.name": params.table, + "clickhouse.request.format": format, + "clickhouse.request.query_id": queryParams.query_id, + "clickhouse.request.session_id": queryParams.session_id, + // Only known up front for array-based inserts; for streamed + // inserts, the row count is not observable by the client. + "clickhouse.request.sent_rows": Array.isArray(params.values) + ? params.values.length + : undefined, + }, + params, + query, + ), }, async (span) => { try { @@ -516,9 +548,12 @@ export class ClickHouseClient { ClickHouseSpanNames.ping, { kind: ClickHouseSpanKind.CLIENT, - attributes: this.withBaseSpanAttributes({ - "clickhouse.ping.select": select, - }), + attributes: this.withBaseSpanAttributes( + { + "clickhouse.ping.select": select, + }, + params, + ), }, async (span) => { try { @@ -555,19 +590,48 @@ export class ClickHouseClient { await this.close(); } + /** + * Builds the common span attributes shared by every operation, merging in the + * operation-specific `extra` attributes and any caller-provided + * {@link BaseQueryParams.span_attributes}. Precedence (lowest to highest): + * caller `span_attributes` -> base semantic-convention attributes -> `extra`, + * so core attributes always win over user-provided ones on key collisions. + * + * When {@link BaseClickHouseClientConfigOptions.dangerously_log_query_text} + * is enabled and a `query` is provided, the raw SQL is attached as + * `db.query.text`. + */ private withBaseSpanAttributes( extra: ClickHouseSpanAttributes, + params?: BaseQueryParams, + query?: string, ): ClickHouseSpanAttributes { const url = this.connectionParams.url; - const attrs: ClickHouseSpanAttributes = { - "db.system.name": "clickhouse", - "server.address": url.hostname, - "server.port": getServerPort(url), - "db.namespace": this.connectionParams.database, - }; + const attrs: ClickHouseSpanAttributes = {}; + // User-provided attributes go in first so that core attributes below + // (and the operation-specific `extra`) take precedence on key collisions. + if (params?.span_attributes !== undefined) { + for (const [k, v] of Object.entries(params.span_attributes)) { + // `db.query.text` is reserved: it must only ever be set through the + // `dangerously_log_query_text` path below, so a caller cannot smuggle + // the raw SQL onto the span via span_attributes and bypass the + // safe-by-default guarantee. + if (v !== undefined && k !== "db.query.text") attrs[k] = v; + } + } + attrs["db.system.name"] = "clickhouse"; + attrs["server.address"] = url.hostname; + attrs["server.port"] = getServerPort(url); + attrs["db.namespace"] = this.connectionParams.database; if (this.connectionParams.application_id !== undefined) { attrs["clickhouse.application"] = this.connectionParams.application_id; } + if ( + this.connectionParams.dangerously_log_query_text && + query !== undefined + ) { + attrs["db.query.text"] = query; + } for (const [k, v] of Object.entries(extra)) { if (v !== undefined) attrs[k] = v; } @@ -600,10 +664,13 @@ function getServerPort(url: URL): number { return url.protocol === "https:" ? 443 : 80; } -/** Records HTTP status and `X-ClickHouse-Summary` counters on the span once - * the response (headers) arrived. The summary values are complete only when - * the query was executed with `wait_end_of_query=1`; see - * {@link ClickHouseSummary}. */ +/** Records HTTP status and the `X-ClickHouse-Summary` counters on the span + * once the response (headers) arrived. Every key present in the parsed + * summary is recorded as `clickhouse.summary.` - the set is not + * hardcoded, so server-side additions (e.g. `memory_usage`, + * `real_time_microseconds`) are picked up automatically. The summary values + * are complete only when the query was executed with `wait_end_of_query=1`; + * see {@link ClickHouseSummary}. */ function setResponseSpanAttributes( span: ClickHouseSpan, result: WithHttpStatusCode & WithClickHouseSummary, @@ -614,13 +681,18 @@ function setResponseSpanAttributes( } const summary = result.summary; if (summary !== undefined) { - attributes["clickhouse.summary.read_rows"] = summary.read_rows; - attributes["clickhouse.summary.read_bytes"] = summary.read_bytes; - attributes["clickhouse.summary.written_rows"] = summary.written_rows; - attributes["clickhouse.summary.written_bytes"] = summary.written_bytes; - attributes["clickhouse.summary.result_rows"] = summary.result_rows; - attributes["clickhouse.summary.result_bytes"] = summary.result_bytes; - attributes["clickhouse.summary.elapsed_ns"] = summary.elapsed_ns; + for (const [key, value] of Object.entries(summary)) { + // `ClickHouseSummary` values are strings; guard against unexpected + // non-primitive shapes so a malformed header can never break tracing. + if ( + value !== undefined && + (typeof value === "string" || + typeof value === "number" || + typeof value === "boolean") + ) { + attributes[`clickhouse.summary.${key}`] = value; + } + } } span.setAttributes(attributes); } diff --git a/packages/client-common/src/config.ts b/packages/client-common/src/config.ts index 44f4c645f..a72b1514c 100644 --- a/packages/client-common/src/config.ts +++ b/packages/client-common/src/config.ts @@ -109,6 +109,22 @@ export interface BaseClickHouseClientConfigOptions { /** The name of the application using the JS client. * @default empty string */ application?: string; + /** + * DANGEROUS: when enabled, the raw SQL query text is attached to tracing + * spans as the OpenTelemetry `db.query.text` attribute and included in the + * `error`-level logs emitted when a request fails. + * + * The query text may contain sensitive data inlined as literals (PII, + * secrets, etc.), which is why this is off by default. Note that bound + * {@link BaseQueryParams.query_params} values and credentials are **never** + * logged or traced, regardless of this setting. + * + * Only enable this if your tracing/logging backend is trusted and you + * understand the implications of persisting query text there. + * + * @default false + */ + dangerously_log_query_text?: boolean; /** Database name to use. * @default default */ database?: string; @@ -392,6 +408,9 @@ export function getConnectionParams( ...(config.use_multipart_params_auto ? { use_multipart_params_auto: true } : {}), + ...(config.dangerously_log_query_text + ? { dangerously_log_query_text: true } + : {}), }; } diff --git a/packages/client-common/src/connection.ts b/packages/client-common/src/connection.ts index 5644dc5a4..7786fe34f 100644 --- a/packages/client-common/src/connection.ts +++ b/packages/client-common/src/connection.ts @@ -22,6 +22,8 @@ export interface ConnectionParams { log_level: ClickHouseLogLevel; keep_alive: { enabled: boolean }; application_id?: string; + /** See {@link BaseClickHouseClientConfigOptions.dangerously_log_query_text}. */ + dangerously_log_query_text?: boolean; http_headers?: Record; auth: ConnectionAuth; json?: JSONHandling; @@ -98,7 +100,8 @@ export interface ConnBaseResult query_id: string; } -export interface ConnQueryResult extends ConnBaseResult { +export interface ConnQueryResult + extends ConnBaseResult, WithClickHouseSummary { stream: Stream; query_id: string; } diff --git a/packages/client-common/src/data_formatter/format_query_params.ts b/packages/client-common/src/data_formatter/format_query_params.ts index 136cbf622..d82c49ec0 100644 --- a/packages/client-common/src/data_formatter/format_query_params.ts +++ b/packages/client-common/src/data_formatter/format_query_params.ts @@ -80,6 +80,14 @@ function formatQueryParamsInternal({ } if (value instanceof Date) { + if (isInArrayOrTuple) { + // Inside a container each element is parsed by the element type's own + // parser: Array(Date)/Array(Date32) reject a bare Unix timestamp and only + // accept a quoted date string. A quoted UTC 'YYYY-MM-DD' is the single + // representation accepted by every temporal element type (Date, Date32, + // DateTime, DateTime64). + return `'${value.toISOString().slice(0, 10)}'`; + } // The ClickHouse server parses numbers as time-zone-agnostic Unix timestamps const unixTimestamp = Math.floor(value.getTime() / 1000) .toString() diff --git a/packages/client-node/CHANGELOG.md b/packages/client-node/CHANGELOG.md index faa822daa..435f9b73f 100644 --- a/packages/client-node/CHANGELOG.md +++ b/packages/client-node/CHANGELOG.md @@ -1,3 +1,23 @@ +# 1.24.0 + +## New features + +- OpenTelemetry: richer, more actionable spans (requested by Langfuse, [#948]). ([#950]) + - The `X-ClickHouse-Summary` counters are now attached to the `clickhouse.query` span as well (previously only on `command` / `exec` / `insert`), so the request-level metrics are visible on the outer client span. + - Every key present in the summary header is now recorded as `clickhouse.summary.` (the set is no longer a hardcoded subset). This surfaces `total_rows_to_read` (already typed but never recorded) and `memory_usage` (peak query memory, in bytes; sent by newer servers), and picks up future server-side additions (e.g. `real_time_microseconds`) automatically. + - `db.response.returned_rows` is now recorded for non-streaming result consumption too (`json()` on `JSON` / `JSONObjectEachRow` / the other single-document JSON formats), not just row-streaming paths. + - Added a `span_attributes` field to the per-request query params (`query` / `command` / `exec` / `insert` / `ping`). Use it to enrich the operation span with application-level context — e.g. mirroring the tags you also send via the `log_comment` setting (route, tenant, surface, etc.). Caller-provided attributes never override the client's own `db.*` / `server.*` / `clickhouse.*` attributes. + - Added a `dangerously_log_query_text` client option (default `false`). When enabled, the raw SQL statement is attached to spans as the OpenTelemetry `db.query.text` attribute and included in the `error`-level logs emitted for a failed request. The query text may contain sensitive literals, which is why it is off by default; bound `query_params` values and credentials are **never** logged or traced regardless of this setting. + +[#948]: https://github.com/ClickHouse/clickhouse-js/issues/948 +[#950]: https://github.com/ClickHouse/clickhouse-js/pull/950 + +## Bug fixes + +- Fixed `Array(Date)` / `Array(Date32)` query-parameter binding (and other temporal element types nested in arrays, tuples, and maps). A JS `Date` inside a container was serialized as a bare Unix timestamp (e.g. `[1683244800]`), which the server's `Array(Date)` element parser rejects (`CANNOT_PARSE_INPUT_ASSERTION_FAILED`). Container-nested `Date` values are now emitted as a quoted UTC date string (e.g. `['2023-05-05']`), the one encoding every temporal element type accepts. Note: a `Date` used inside `Array(DateTime)` / `Array(DateTime64)` is now bound at day precision (the time-of-day is dropped), since date-only is the only form `Array(Date)` accepts; scalar `Date` / `DateTime` binding is unchanged. ([#947]) + +[#947]: https://github.com/ClickHouse/clickhouse-js/pull/947 + # 1.23.1 ## Bug Fixes diff --git a/packages/client-node/__tests__/unit/node_log_query_text.test.ts b/packages/client-node/__tests__/unit/node_log_query_text.test.ts new file mode 100644 index 000000000..077fa3ec5 --- /dev/null +++ b/packages/client-node/__tests__/unit/node_log_query_text.test.ts @@ -0,0 +1,172 @@ +import { describe, it, expect, beforeAll, afterAll, beforeEach } from "vitest"; +import Http from "node:http"; +import { AddressInfo } from "node:net"; +import type Stream from "stream"; +import type { + ClickHouseClient, + ErrorLogParams, + Logger, + LogParams, +} from "@clickhouse/client-common"; +import { ClickHouseLogLevel } from "@clickhouse/client-common"; +import { createSimpleNodeTestClient } from "../utils/simple_node_client"; + +// The client must never log the bound query parameter values or the +// credentials. The raw SQL text is logged ONLY when the caller explicitly +// opts in via `dangerously_log_query_text`; it is scrubbed by default. +// The connection layer surfaces request errors via `log_writer.error`, so we +// drive a failing request and inspect the captured log entries. +const SECRET_SQL_MARKER = "topsecret_sql_marker"; +const SECRET_PARAM_MARKER = "topsecret_param_marker"; +const SECRET_PASSWORD_MARKER = "topsecret_password_marker"; + +interface CapturedLog { + message: string; + args?: Record; + err?: Error; +} + +const capturedErrors: CapturedLog[] = []; + +class CapturingLogger implements Logger { + trace(_params: LogParams): void {} + debug(_params: LogParams): void {} + info(_params: LogParams): void {} + warn(_params: LogParams): void {} + error({ message, args, err }: ErrorLogParams): void { + capturedErrors.push({ message, args, err }); + } +} + +describe("[Node.js] query text in logs", () => { + let server: Http.Server; + let port: number; + + function makeClient( + dangerously_log_query_text: boolean, + ): ClickHouseClient { + return createSimpleNodeTestClient({ + url: `http://127.0.0.1:${port}`, + username: "default", + password: SECRET_PASSWORD_MARKER, + dangerously_log_query_text, + log: { LoggerClass: CapturingLogger, level: ClickHouseLogLevel.ERROR }, + }) as unknown as ClickHouseClient; + } + + beforeAll(async () => { + // Immediately reset every incoming connection so the outgoing request + // fails and the connection layer reaches its error-logging branch. + server = Http.createServer(); + server.on("connection", (socket) => socket.destroy()); + await new Promise((resolve) => + server.listen(0, "127.0.0.1", resolve), + ); + port = (server.address() as AddressInfo).port; + }); + + beforeEach(() => { + capturedErrors.length = 0; + }); + + afterAll(async () => { + await new Promise((resolve, reject) => + server.close((err) => (err ? reject(err) : resolve())), + ); + }); + + function serializedArgs(): string { + return JSON.stringify( + capturedErrors.map(({ message, args }) => ({ message, args })), + ); + } + + function assertLoggedButNoSecrets(shouldContainSql: boolean) { + // At least one error must have been logged so the assertions are meaningful. + expect(capturedErrors.length).toBeGreaterThan(0); + const serialized = serializedArgs(); + // Param values and credentials are never logged, in either mode. + expect(serialized).not.toContain(SECRET_PARAM_MARKER); + expect(serialized).not.toContain(SECRET_PASSWORD_MARKER); + if (shouldContainSql) { + expect(serialized).toContain(SECRET_SQL_MARKER); + } else { + expect(serialized).not.toContain(SECRET_SQL_MARKER); + } + // Sanity check: non-sensitive diagnostics are still logged. + for (const { args } of capturedErrors) { + expect(args).toBeDefined(); + expect(args).toHaveProperty("query_id"); + expect(args).toHaveProperty("with_abort_signal"); + } + } + + describe("with dangerously_log_query_text disabled (default)", () => { + let client: ClickHouseClient; + beforeAll(() => { + client = makeClient(false); + }); + afterAll(async () => { + await client.close(); + }); + + it("does not log SQL/params/credentials on a failed query", async () => { + await expect( + client.query({ + query: `SELECT '${SECRET_SQL_MARKER}', {p:String}`, + query_params: { p: SECRET_PARAM_MARKER }, + }), + ).rejects.toThrow(); + assertLoggedButNoSecrets(false); + }); + + it("does not log SQL/params/credentials on a failed insert", async () => { + await expect( + client.insert({ + table: SECRET_SQL_MARKER, + values: [{ id: SECRET_PARAM_MARKER }], + format: "JSONEachRow", + }), + ).rejects.toThrow(); + assertLoggedButNoSecrets(false); + }); + + it("does not log SQL/params/credentials on a failed command", async () => { + await expect( + client.command({ + query: `CREATE TABLE ${SECRET_SQL_MARKER} (id String) ENGINE Null`, + }), + ).rejects.toThrow(); + assertLoggedButNoSecrets(false); + }); + }); + + describe("with dangerously_log_query_text enabled", () => { + let client: ClickHouseClient; + beforeAll(() => { + client = makeClient(true); + }); + afterAll(async () => { + await client.close(); + }); + + it("logs the SQL text (but never params/credentials) on a failed query", async () => { + await expect( + client.query({ + query: `SELECT '${SECRET_SQL_MARKER}', {p:String}`, + query_params: { p: SECRET_PARAM_MARKER }, + }), + ).rejects.toThrow(); + assertLoggedButNoSecrets(true); + }); + + it("logs the SQL text on a failed command", async () => { + await expect( + client.command({ + query: `CREATE TABLE ${SECRET_SQL_MARKER} (id String) ENGINE Null`, + }), + ).rejects.toThrow(); + assertLoggedButNoSecrets(true); + }); + }); +}); diff --git a/packages/client-node/__tests__/unit/node_result_set_span.test.ts b/packages/client-node/__tests__/unit/node_result_set_span.test.ts index a0b1d65b7..ce2d60dba 100644 --- a/packages/client-node/__tests__/unit/node_result_set_span.test.ts +++ b/packages/client-node/__tests__/unit/node_result_set_span.test.ts @@ -67,6 +67,44 @@ describe("[Node.js] ResultSet span tracking", () => { expect(span.status).toBeUndefined(); }); + it("records returned_rows for the non-streaming JSON format via json()", async () => { + const span = new RecordedSpan(); + const body = JSON.stringify({ + meta: [{ name: "n", type: "UInt8" }], + data: [{ n: 1 }, { n: 2 }, { n: 3 }], + rows: 3, + }); + const rs = new ResultSet( + Readable.from([Buffer.from(body)]), + "JSON", + "query-id", + undefined, + undefined, + undefined, + span, + ); + await rs.json(); + expect(span.endedTimes).toBe(1); + expect(span.attributes["db.response.returned_rows"]).toBe(3); + }); + + it("records returned_rows for JSONObjectEachRow via json()", async () => { + const span = new RecordedSpan(); + const body = JSON.stringify({ row_1: { n: 1 }, row_2: { n: 2 } }); + const rs = new ResultSet( + Readable.from([Buffer.from(body)]), + "JSONObjectEachRow", + "query-id", + undefined, + undefined, + undefined, + span, + ); + await rs.json(); + expect(span.endedTimes).toBe(1); + expect(span.attributes["db.response.returned_rows"]).toBe(2); + }); + it("ends the span and records rows + bytes after the stream is fully consumed", async () => { const span = new RecordedSpan(); const rs = makeResultSet(span); diff --git a/packages/client-node/package.json b/packages/client-node/package.json index f9c6b5d7a..463268a23 100644 --- a/packages/client-node/package.json +++ b/packages/client-node/package.json @@ -2,7 +2,7 @@ "name": "@clickhouse/client", "description": "Official JS client for ClickHouse DB - Node.js implementation", "homepage": "https://clickhouse.com", - "version": "1.23.1", + "version": "1.24.0", "license": "Apache-2.0", "keywords": [ "clickhouse", diff --git a/packages/client-node/src/connection/node_base_connection.ts b/packages/client-node/src/connection/node_base_connection.ts index 3608ff532..7ab67c7b9 100644 --- a/packages/client-node/src/connection/node_base_connection.ts +++ b/packages/client-node/src/connection/node_base_connection.ts @@ -244,23 +244,26 @@ export abstract class NodeBaseConnection implements Connection } try { - const { response_headers, stream, http_status_code } = await this.request( - { - method: "POST", - url: transformUrl({ url: this.params.url, searchParams }), - body, - abort_signal: controller.signal, - response_compression_codec: responseCompressionCodec, - headers, - query: params.query, - query_id, - log_writer, - log_level, - }, - "Query", - ); + const { response_headers, stream, summary, http_status_code } = + await this.request( + { + method: "POST", + url: transformUrl({ url: this.params.url, searchParams }), + body, + abort_signal: controller.signal, + response_compression_codec: responseCompressionCodec, + parse_summary: true, + headers, + query: params.query, + query_id, + log_writer, + log_level, + }, + "Query", + ); return { stream, + summary, response_headers, query_id, http_status_code, @@ -270,8 +273,9 @@ export abstract class NodeBaseConnection implements Connection this.logRequestError({ op: "Query", query_id: query_id, - query_params: params, - search_params: searchParams, + session_id: params.session_id, + with_abort_signal: params.abort_signal !== undefined, + query: params.query, err: err as Error, extra_args: { decompress_response: responseCompressionCodec, @@ -332,8 +336,9 @@ export abstract class NodeBaseConnection implements Connection this.logRequestError({ op: "Insert", query_id: query_id, - query_params: params, - search_params: searchParams, + session_id: params.session_id, + with_abort_signal: params.abort_signal !== undefined, + query: params.query, err: err as Error, extra_args: { clickhouse_settings: params.clickhouse_settings ?? {}, @@ -505,7 +510,9 @@ export abstract class NodeBaseConnection implements Connection op, err, query_id, - query_params, + session_id, + with_abort_signal, + query, extra_args, }: LogRequestErrorParams) { if (this.params.log_level <= ClickHouseLogLevel.ERROR) { @@ -516,8 +523,14 @@ export abstract class NodeBaseConnection implements Connection operation: op, connection_id: this.connectionId, query_id, - with_abort_signal: query_params.abort_signal !== undefined, - session_id: query_params.session_id, + with_abort_signal, + session_id, + // The raw SQL is only ever logged when the caller has explicitly + // opted in. Bound `query_params` values and credentials are never + // logged, regardless of this setting. + ...(this.params.dangerously_log_query_text && query !== undefined + ? { query } + : {}), ...extra_args, }, }); @@ -592,8 +605,9 @@ export abstract class NodeBaseConnection implements Connection this.logRequestError({ op: params.op, query_id: query_id, - query_params: params, - search_params: searchParams, + session_id: params.session_id, + with_abort_signal: params.abort_signal !== undefined, + query: params.query, err: err as Error, extra_args: { clickhouse_settings: params.clickhouse_settings ?? {}, @@ -624,8 +638,12 @@ interface LogRequestErrorParams { op: ConnOperation; err: Error; query_id: string; - query_params: ConnBaseQueryParams; - search_params: URLSearchParams | undefined; + session_id: string | undefined; + with_abort_signal: boolean; + /** Raw SQL text. Passed through unconditionally by callers, but only ever + * written to the log when + * {@link ConnectionParams.dangerously_log_query_text} is enabled. */ + query: string | undefined; extra_args: Record; } diff --git a/packages/client-node/src/result_set.ts b/packages/client-node/src/result_set.ts index 6c1f6347c..cb9f827eb 100644 --- a/packages/client-node/src/result_set.ts +++ b/packages/client-node/src/result_set.ts @@ -19,6 +19,7 @@ import { import { isNotStreamableJSONFamily, isStreamableJSONFamily, + RecordsJSONFormats, validateStreamFormat, } from "./common/index"; import { Buffer } from "buffer"; @@ -193,8 +194,16 @@ export class ResultSet< try { const text = await getAsText(stream); this.span_bytes += Buffer.byteLength(text); + const parsed = this.jsonHandling.parse>(text); + // Record the returned row count for non-streaming consumption too, so + // `db.response.returned_rows` is present regardless of the format / + // consumption style (see countReturnedRows). + const rows = countReturnedRows(parsed, this.format as DataFormat); + if (rows !== undefined) { + this.addSpanRows(rows); + } this.finishSpan(); - return this.jsonHandling.parse(text); + return parsed; } catch (err) { this.finishSpan(err); throw err; @@ -337,3 +346,30 @@ export class ResultSet< const streamAlreadyConsumedMessage = "Stream has been already consumed"; const resultSetClosedMessage = "ResultSet has been closed"; + +/** Best-effort row count for a fully-parsed non-streaming JSON document, used + * to populate `db.response.returned_rows` for `json()` consumption. Returns + * `undefined` when the shape is unrecognized (the attribute is then omitted). + * - `JSONObjectEachRow` (and other record formats) parse to `Record` + * -> the number of keys. + * - `JSON`, `JSONCompact`, etc. parse to `{ data: T[], rows: number, ... }` + * -> the reported `rows`, falling back to `data.length`. */ +function countReturnedRows( + parsed: unknown, + format: DataFormat, +): number | undefined { + if (parsed === null || typeof parsed !== "object") { + return undefined; + } + if ((RecordsJSONFormats as readonly string[]).includes(format)) { + return Object.keys(parsed).length; + } + const obj = parsed as { rows?: unknown; data?: unknown }; + if (typeof obj.rows === "number") { + return obj.rows; + } + if (Array.isArray(obj.data)) { + return obj.data.length; + } + return undefined; +} diff --git a/packages/client-node/src/version.ts b/packages/client-node/src/version.ts index aace03d41..41511d0bd 100644 --- a/packages/client-node/src/version.ts +++ b/packages/client-node/src/version.ts @@ -1 +1 @@ -export default "1.23.1"; +export default "1.24.0"; diff --git a/packages/client-web/CHANGELOG.md b/packages/client-web/CHANGELOG.md index 1883e1f6d..12fb839c6 100644 --- a/packages/client-web/CHANGELOG.md +++ b/packages/client-web/CHANGELOG.md @@ -1,3 +1,23 @@ +# 1.24.0 + +## New features + +- OpenTelemetry: richer, more actionable spans (requested by Langfuse, [#948]). ([#950]) + - The `X-ClickHouse-Summary` header is now parsed on the Web client and attached to the operation spans (`query` / `command` / `exec` / `insert`), including the outer `clickhouse.query` span. Previously the Web client did not parse the summary header at all. + - Every key present in the summary header is recorded as `clickhouse.summary.` (the set is not hardcoded). This surfaces `total_rows_to_read` and `memory_usage` (peak query memory, in bytes; sent by newer servers), and picks up future server-side additions (e.g. `real_time_microseconds`) automatically. + - `db.response.returned_rows` is now recorded for non-streaming result consumption too (`json()` on `JSON` / `JSONObjectEachRow` / the other single-document JSON formats), not just row-streaming paths. + - Added a `span_attributes` field to the per-request query params (`query` / `command` / `exec` / `insert` / `ping`). Use it to enrich the operation span with application-level context — e.g. mirroring the tags you also send via the `log_comment` setting (route, tenant, surface, etc.). Caller-provided attributes never override the client's own `db.*` / `server.*` / `clickhouse.*` attributes. + - Added a `dangerously_log_query_text` client option (default `false`). When enabled, the raw SQL statement is attached to spans as the OpenTelemetry `db.query.text` attribute. The query text may contain sensitive literals, which is why it is off by default; bound `query_params` values and credentials are **never** traced regardless of this setting. + +[#948]: https://github.com/ClickHouse/clickhouse-js/issues/948 +[#950]: https://github.com/ClickHouse/clickhouse-js/pull/950 + +## Bug fixes + +- Fixed `Array(Date)` / `Array(Date32)` query-parameter binding (and other temporal element types nested in arrays, tuples, and maps). A JS `Date` inside a container was serialized as a bare Unix timestamp (e.g. `[1683244800]`), which the server's `Array(Date)` element parser rejects (`CANNOT_PARSE_INPUT_ASSERTION_FAILED`). Container-nested `Date` values are now emitted as a quoted UTC date string (e.g. `['2023-05-05']`), the one encoding every temporal element type accepts. Note: a `Date` used inside `Array(DateTime)` / `Array(DateTime64)` is now bound at day precision (the time-of-day is dropped), since date-only is the only form `Array(Date)` accepts; scalar `Date` / `DateTime` binding is unchanged. ([#947]) + +[#947]: https://github.com/ClickHouse/clickhouse-js/pull/947 + # 1.23.1 ## Bug Fixes diff --git a/packages/client-web/__tests__/unit/web_summary.test.ts b/packages/client-web/__tests__/unit/web_summary.test.ts new file mode 100644 index 000000000..2e076bf63 --- /dev/null +++ b/packages/client-web/__tests__/unit/web_summary.test.ts @@ -0,0 +1,95 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { ClickHouseLogLevel, LogWriter } from "@clickhouse/client-common"; +import { TestLogger } from "../../../client-common/__tests__/utils/test_logger"; +import { WebConnection, type WebConnectionParams } from "../../src/connection"; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +const SUMMARY = { + read_rows: "6568", + read_bytes: "3894304", + written_rows: "0", + written_bytes: "0", + total_rows_to_read: "6568", + result_rows: "0", + result_bytes: "0", + elapsed_ns: "10693523", + memory_usage: "4580679", +}; + +function stubFetch(headers: Record) { + return vi.fn(async () => ({ + status: 200, + body: null, + text: async () => "", + headers: new Headers({ + "x-clickhouse-query-id": "test-query-id", + ...headers, + }), + })) as unknown as ReturnType & typeof fetch; +} + +function buildWebConnection(fetch: typeof fetch): WebConnection { + return new WebConnection({ + url: new URL("https://localhost:8443"), + request_timeout: 30_000, + compression: { + decompress_response: undefined, + compress_request: undefined, + }, + max_open_connections: 10, + auth: { username: "default", password: "", type: "Credentials" }, + database: "default", + clickhouse_settings: {}, + log_writer: new LogWriter( + new TestLogger(), + "WebSummaryTest", + ClickHouseLogLevel.OFF, + ), + log_level: ClickHouseLogLevel.OFF, + keep_alive: { enabled: false }, + fetch, + } as WebConnectionParams); +} + +describe("[Web] X-ClickHouse-Summary parsing", () => { + it("parses the summary header for query()", async () => { + const conn = buildWebConnection( + stubFetch({ "x-clickhouse-summary": JSON.stringify(SUMMARY) }), + ); + const result = await conn.query({ query: "SELECT 1" }); + expect(result.summary).toEqual(SUMMARY); + }); + + it("parses the summary header for insert()", async () => { + const conn = buildWebConnection( + stubFetch({ "x-clickhouse-summary": JSON.stringify(SUMMARY) }), + ); + const result = await conn.insert({ query: "INSERT INTO t", values: "" }); + expect(result.summary).toEqual(SUMMARY); + }); + + it("parses the summary header for command()", async () => { + const conn = buildWebConnection( + stubFetch({ "x-clickhouse-summary": JSON.stringify(SUMMARY) }), + ); + const result = await conn.command({ query: "CREATE TABLE t (a UInt8)" }); + expect(result.summary).toEqual(SUMMARY); + }); + + it("returns undefined summary when the header is absent", async () => { + const conn = buildWebConnection(stubFetch({})); + const result = await conn.query({ query: "SELECT 1" }); + expect(result.summary).toBeUndefined(); + }); + + it("returns undefined summary (and does not throw) when the header is malformed", async () => { + const conn = buildWebConnection( + stubFetch({ "x-clickhouse-summary": "not-json{" }), + ); + const result = await conn.query({ query: "SELECT 1" }); + expect(result.summary).toBeUndefined(); + }); +}); diff --git a/packages/client-web/package.json b/packages/client-web/package.json index cfc36bbc7..f3ba5c021 100644 --- a/packages/client-web/package.json +++ b/packages/client-web/package.json @@ -2,7 +2,7 @@ "name": "@clickhouse/client-web", "description": "Official JS client for ClickHouse DB - Web API implementation", "homepage": "https://clickhouse.com", - "version": "1.23.1", + "version": "1.24.0", "license": "Apache-2.0", "keywords": [ "clickhouse", diff --git a/packages/client-web/src/connection/web_connection.ts b/packages/client-web/src/connection/web_connection.ts index 3698059be..32ddb08d9 100644 --- a/packages/client-web/src/connection/web_connection.ts +++ b/packages/client-web/src/connection/web_connection.ts @@ -1,4 +1,5 @@ import type { + ClickHouseSummary, ConnBaseQueryParams, ConnCommandResult, Connection, @@ -113,6 +114,7 @@ export class WebConnection implements Connection { return { query_id, stream: response.body || new ReadableStream(), + summary: this.getSummary(response), response_headers: getResponseHeaders(response), http_status_code: response.status, }; @@ -125,18 +127,19 @@ export class WebConnection implements Connection { return { query_id: result.query_id, stream: result.stream || new ReadableStream(), + summary: result.summary, response_headers: result.response_headers, http_status_code: result.http_status_code, }; } async command(params: ConnBaseQueryParams): Promise { - const { stream, query_id, response_headers, http_status_code } = + const { stream, query_id, summary, response_headers, http_status_code } = await this.runExec(params); if (stream !== null) { await stream.cancel(); } - return { query_id, response_headers, http_status_code }; + return { query_id, summary, response_headers, http_status_code }; } async insert( @@ -162,6 +165,7 @@ export class WebConnection implements Connection { } return { query_id, + summary: this.getSummary(response), response_headers: getResponseHeaders(response), http_status_code: response.status, }; @@ -301,12 +305,38 @@ export class WebConnection implements Connection { }); return { stream: response.body, + summary: this.getSummary(response), response_headers: getResponseHeaders(response), query_id, http_status_code: response.status, }; } + /** Parse the `X-ClickHouse-Summary` response header into a + * {@link ClickHouseSummary}, or `undefined` if the header is absent or + * cannot be parsed. Parsing failures are logged and swallowed - the summary + * is optional diagnostic metadata and must never fail a request. */ + private getSummary(response: Response): ClickHouseSummary | undefined { + const summaryHeader = response.headers.get("x-clickhouse-summary"); + if (summaryHeader === null) { + return undefined; + } + const parse = this.params.json?.parse ?? JSON.parse; + try { + return parse(summaryHeader) as ClickHouseSummary; + } catch (err) { + this.params.log_writer.warn({ + module: "WebConnection", + message: "Failed to parse X-ClickHouse-Summary header.", + args: { + "X-ClickHouse-Summary": summaryHeader, + error: String(err), + }, + }); + return undefined; + } + } + private defaultHeadersWithOverride( params?: ConnBaseQueryParams, ): Record { @@ -343,6 +373,7 @@ function getResponseHeaders(response: Response): ResponseHeaders { interface RunExecResult { stream: ReadableStream | null; query_id: string; + summary?: ClickHouseSummary; response_headers: ResponseHeaders; http_status_code: number; } diff --git a/packages/client-web/src/result_set.ts b/packages/client-web/src/result_set.ts index 8a1fb1578..d4f7fd338 100644 --- a/packages/client-web/src/result_set.ts +++ b/packages/client-web/src/result_set.ts @@ -17,6 +17,7 @@ import { import { isNotStreamableJSONFamily, isStreamableJSONFamily, + RecordsJSONFormats, validateStreamFormat, } from "./common/index"; import { getAsText } from "./utils"; @@ -129,8 +130,16 @@ export class ResultSet< // Same as text(): record text.length (UTF-16 code-unit count) rather // than span_bytes to avoid the TextEncoder allocation. this.span_text_length = text.length; + const parsed = this.jsonHandling.parse>(text); + // Record the returned row count for non-streaming consumption too, so + // `db.response.returned_rows` is present regardless of the format / + // consumption style (see countReturnedRows). + const rows = countReturnedRows(parsed, this.format as DataFormat); + if (rows !== undefined) { + this.addSpanRows(rows); + } this.finishSpan(); - return this.jsonHandling.parse(text); + return parsed; } catch (err) { this.finishSpan(err); throw err; @@ -320,3 +329,30 @@ export class ResultSet< } const streamAlreadyConsumedMessage = "Stream has been already consumed"; + +/** Best-effort row count for a fully-parsed non-streaming JSON document, used + * to populate `db.response.returned_rows` for `json()` consumption. Returns + * `undefined` when the shape is unrecognized (the attribute is then omitted). + * - `JSONObjectEachRow` (and other record formats) parse to `Record` + * -> the number of keys. + * - `JSON`, `JSONCompact`, etc. parse to `{ data: T[], rows: number, ... }` + * -> the reported `rows`, falling back to `data.length`. */ +function countReturnedRows( + parsed: unknown, + format: DataFormat, +): number | undefined { + if (parsed === null || typeof parsed !== "object") { + return undefined; + } + if ((RecordsJSONFormats as readonly string[]).includes(format)) { + return Object.keys(parsed).length; + } + const obj = parsed as { rows?: unknown; data?: unknown }; + if (typeof obj.rows === "number") { + return obj.rows; + } + if (Array.isArray(obj.data)) { + return obj.data.length; + } + return undefined; +} diff --git a/packages/client-web/src/version.ts b/packages/client-web/src/version.ts index aace03d41..41511d0bd 100644 --- a/packages/client-web/src/version.ts +++ b/packages/client-web/src/version.ts @@ -1 +1 @@ -export default "1.23.1"; +export default "1.24.0";