DT-61: Implement ClickHouse client and create necessary tables and views by abdotop · Pull Request #62 · 01-edu/devtools

abdotop · 2025-08-29T10:16:09Z

No description provided.

kigiri · 2025-08-29T10:22:30Z

api/click-house-client.ts

+    query: `
+      CREATE TABLE IF NOT EXISTS logs (
+        deployment_id LowCardinality(String),
+        timestamp DateTime64(3, 'UTC') DEFAULT now64(3, 'UTC'),


kigiri · 2025-08-29T10:30:08Z

api/click-house-client.ts

+        INDEX idx_message message TYPE tokenbf_v1(8192, 3, 0) GRANULARITY 4
+      )
+      ENGINE = MergeTree
+      PARTITION BY toYYYYMMDD(timestamp)


I want to know more about this

…bles and views

…nsertion endpoint

…ession handling

Copilot

Pull Request Overview

This pull request implements ClickHouse integration for log storage and management, adding database client configuration and deployment token-based authentication for log ingestion.

Adds ClickHouse client with table creation for structured log storage
Implements deployment management API endpoints with encrypted token authentication
Integrates ClickHouse setup into the development and production workflows

Reviewed Changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tasks/clickhouse.ts	Creates ClickHouse logs table with proper schema and partitioning
deno.json	Adds ClickHouse dependency and integrates setup tasks into dev/prod workflows
api/user.ts	Exports encryption functions for deployment token generation
api/server.ts	Adds resource field to request context for deployment tracking
api/routes.ts	Implements complete deployment CRUD API and log ingestion endpoint
api/lib/validator.ts	Adds UNION validator for handling multiple input types
api/lib/env.ts	Adds required ClickHouse environment variable validation
api/lib/context.ts	Extends request context with resource tracking
api/click-house-client.ts	Implements ClickHouse client and log insertion functionality
.env.test	Adds ClickHouse configuration for test environment

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-08-30T16:52:49Z

api/routes.ts

+      return deployments.map(({ tokenSalt: _, ...d }) => {
+        return {
+          ...d,
+          createdAt: d.createdAt,
+          updatedAt: d.updatedAt,
+          token: undefined,
+          sqlToken: undefined,
+          sqlEndpoint: undefined,
+        }
+      })


The explicit assignment of createdAt and updatedAt fields is redundant since they're already included in the spread operator ...d. These lines can be removed.

Copilot · 2025-08-30T16:52:49Z

api/routes.ts

+        createdAt: deployment.createdAt,
+        updatedAt: deployment.updatedAt,


The explicit assignment of createdAt and updatedAt fields is redundant since they're already included in the spread operator ...deployment. These lines can be removed.

Copilot · 2025-08-30T16:52:49Z

api/routes.ts

+        createdAt: deployment.createdAt,
+        updatedAt: deployment.updatedAt,


The explicit assignment of createdAt and updatedAt fields is redundant since they're already included in the spread operator ...deployment. These lines can be removed.

Copilot · 2025-08-30T16:52:50Z

api/routes.ts

+        createdAt: deployment.createdAt,
+        updatedAt: deployment.updatedAt,


The explicit assignment of createdAt and updatedAt fields is redundant since they're already included in the spread operator ...deployment. These lines can be removed.

Copilot · 2025-08-30T16:52:50Z

api/routes.ts

+        createdAt: deployment.createdAt,
+        updatedAt: deployment.updatedAt,


The explicit assignment of createdAt and updatedAt fields is redundant since they're already included in the spread operator ...deployment. These lines can be removed.

kigiri · 2025-08-31T23:10:59Z

api/click-house-client.ts

+  severity_text: STR(),
+  severity_number: NUM(),
+  attributes: OBJ({}),
+  event_name: STR(),


describe each fields, isn't severity_text & severity_number redundant ? do we ever use severity_number ?

…y_text

…and update log insertion functionality

kigiri · 2025-09-03T15:50:14Z

api/routes.ts

+        return {
+          ...d,
+          createdAt: d.createdAt,
+          updatedAt: d.updatedAt,


should be fixed with a type assertion, not JS (IMO)

kigiri · 2025-09-03T16:45:27Z

tasks/clickhouse.ts

+        timestamp DateTime64(3, 'UTC') DEFAULT now64(3, 'UTC'),
+        observed_timestamp DateTime64(3, 'UTC') DEFAULT now64(3, 'UTC'),
+        trace_id UInt64,
+        span_id UInt64,


trace_id & span_id should be FixedString(16) as per Otel spec:
https://opentelemetry.io/docs/specs/otel/logs/data-model/#trace-context-fields

To convert from our internal float64 id to their 128bit id we can do this:

function float64ToId128(id) { const id128 = new Uint8Array(16) crypto.getRandomValues(id128) // prefill with random values const view = new DataView(id128.buffer) view.setFloat64(8, id, false) return id128 }

kigiri · 2025-09-03T16:50:31Z

tasks/clickhouse.ts

+        observed_timestamp DateTime64(3, 'UTC') DEFAULT now64(3, 'UTC'),
+        trace_id UInt64,
+        span_id UInt64,
+        severity_number UInt8,


we could store also the severity_string in the db and derive it from the severity_number just not take it as a parameter to the route, or do the oposite

kigiri · 2025-09-03T17:23:29Z

tasks/clickhouse.ts

+        severity_number UInt8,
+        attributes JSON,
+        event_name String,
+        context JSON


remove context and resource
and instead use "flattened" attributes for service

-- Flattened resource fields service_name LowCardinality(String), service_version LowCardinality(String), service_instance_id String,

kigiri · 2025-09-03T17:24:50Z

tasks/clickhouse.ts

+        attributes JSON,
+        event_name String,
+        context JSON
+      )


to be compliant, looks like we need a body, even if we don't use it we can make it nullable:

-- Often empty, but kept for OTEL spec compliance body Nullable(String),

We could try to add support to set it from the API as a special attribute, could be use ingesting log that only have text and no attributes ?

…ata types for better logging structure

kigiri · 2025-09-11T10:39:29Z

api/click-house-client.ts

+
+export function bytesToHex(bytes: Uint8Array) {
+  return Array.from(bytes).map((b) => b.toString(16).padStart(2, '0')).join('')
+}


const numberToHex128 = (() => { const alphabet = new TextEncoder().encode("0123456789abcdef") const output = new Uint8Array(16) const view = new DataView(new Uint8Array(8).buffer) const dec = new TextDecoder() return (id: number) => { view.setFloat64(0, id, false) let i = -1 while (++i < 8) { const x = view.getUint8(i) output[i * 2] = alphabet[x >> 4] output[i * 2 + 1] = alphabet[x & 0xF] } return dec.decode(output) } })()

kigiri · 2025-09-12T10:36:36Z

api/click-house-client.ts

+}
+
+function toClickhouseDateTime(iso: string) {
+  // "2025-09-11T17:35:00.000Z" → "2025-09-11 17:35:00"


"2025-09-11T17:35:00.000Z".replace('T', ' ').slice(0, -5)

even better:
https://github.com/ClickHouse/clickhouse-js/blob/main/examples/insert_js_dates.ts

kigiri · 2025-09-12T10:43:34Z

tasks/clickhouse.ts

+        span_id FixedString(16),
+        severity_number UInt8,
+        -- derived column, computed by DB from severity_number
+        severity_text String MATERIALIZED CASE


should be LowCardinality(String)

kigiri · 2025-09-12T12:58:54Z

api/click-house-client.ts

+  data: LogsInput,
+) {
+  const logsToInsert = Array.isArray(data) ? data : [data]
+  if (logsToInsert.length === 0) throw respond.NoContent()


is it ok to throw a response ?

kigiri · 2025-09-12T12:59:20Z

api/click-house-client.ts

+    return respond.OK()
+  } catch (error) {
+    log.error('Error inserting logs into ClickHouse:', { error })
+    throw respond.InternalServerError()


same here, shouldn't we just return in that case ?

… span IDs

kigiri · 2025-09-15T10:44:42Z

web/pages/ProjectsPage.tsx

+  { project, team }: { project: Project; team: Team },
 ) => {
+  console.log(project)
+  console.log(team)


abdotop linked an issue Aug 29, 2025 that may be closed by this pull request

Click-House config for deployments logs #61

Closed

abdotop force-pushed the 61-click-house-config-for-deployments-logs branch from d7d03f7 to c567b7f Compare August 29, 2025 10:17

kigiri reviewed Aug 29, 2025

View reviewed changes

abdotop self-assigned this Aug 30, 2025

abdotop added 2 commits August 30, 2025 14:53

feat(clickhouse): Implement ClickHouse client and create necessary ta…

edf4dbb

…bles and views

feat(logging): Enhance ClickHouse logging functionality and add log i…

27d7232

…nsertion endpoint

abdotop force-pushed the 61-click-house-config-for-deployments-logs branch 2 times, most recently from a9e74ae to 4ea28b7 Compare August 30, 2025 16:31

abdotop requested review from Copilot and kigiri August 30, 2025 16:33

This comment was marked as outdated.

Sign in to view

abdotop force-pushed the 61-click-house-config-for-deployments-logs branch from 4ea28b7 to 8e914e1 Compare August 30, 2025 16:36

abdotop requested a review from Copilot August 30, 2025 16:37

This comment was marked as outdated.

Sign in to view

abdotop force-pushed the 61-click-house-config-for-deployments-logs branch from 8e914e1 to 66fb184 Compare August 30, 2025 16:38

feat(deployment): Add deployment management routes and enhance user s…

014ad5e

…ession handling

abdotop force-pushed the 61-click-house-config-for-deployments-logs branch from 66fb184 to 014ad5e Compare August 30, 2025 16:51

abdotop requested a review from Copilot August 30, 2025 16:51

Copilot AI reviewed Aug 30, 2025

View reviewed changes

kigiri reviewed Aug 31, 2025

View reviewed changes

abdotop added 2 commits September 1, 2025 17:32

feat(schema): Update log schema to include context and remove severit…

770c06c

…y_text

feat(logging): Add getLogs endpoint to retrieve logs from ClickHouse …

b3e0b67

…and update log insertion functionality

kigiri reviewed Sep 3, 2025

View reviewed changes

abdotop force-pushed the 61-click-house-config-for-deployments-logs branch 2 times, most recently from 698a7ca to 913ff62 Compare September 10, 2025 10:01

abdotop force-pushed the 61-click-house-config-for-deployments-logs branch from 913ff62 to 0e431e1 Compare September 10, 2025 10:12

feat(schema): Enhance log schema with additional fields and improve d…

54ee31c

…ata types for better logging structure

abdotop force-pushed the 61-click-house-config-for-deployments-logs branch from 0e431e1 to 54ee31c Compare September 11, 2025 09:54

kigiri reviewed Sep 11, 2025

View reviewed changes

kigiri reviewed Sep 12, 2025

View reviewed changes

abdotop force-pushed the 61-click-house-config-for-deployments-logs branch from e9d522e to 11636bb Compare September 12, 2025 12:49

kigiri reviewed Sep 12, 2025

View reviewed changes

abdotop force-pushed the 61-click-house-config-for-deployments-logs branch 6 times, most recently from 3fb67ff to e1dac52 Compare September 15, 2025 09:51

feat(logging): Refactor number conversion functions for log trace and…

a0e50df

… span IDs

abdotop force-pushed the 61-click-house-config-for-deployments-logs branch from e1dac52 to a0e50df Compare September 15, 2025 09:54

kigiri approved these changes Sep 15, 2025

View reviewed changes

kigiri reviewed Sep 15, 2025

View reviewed changes

web/pages/ProjectsPage.tsx

{ project, team }: { project: Project; team: Team },

) => {

console.log(project)

console.log(team)

Copy link

Member

kigiri Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

???

kigiri marked this pull request as ready for review September 15, 2025 10:45

kigiri merged commit 61c7b47 into master Sep 15, 2025
2 checks passed

kigiri deleted the 61-click-house-config-for-deployments-logs branch September 15, 2025 10:45

		createdAt: deployment.createdAt,
		updatedAt: deployment.updatedAt,

Conversation

abdotop commented Aug 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 30, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kigiri Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kigiri Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kigiri Sep 3, 2025 •

edited

Loading

kigiri Sep 11, 2025 •

edited

Loading