fix: Include connectionId in metadata cache key (#1272)

Closes HDX-2573

This PR adds connection ID to the metadata cache key, to ensure that metadata is not shared across tables with the same name in different connections.

For example, previously when switching between otel_logs tables in different connections, filter values would not reload:

https://github.com/user-attachments/assets/d3edb243-4564-438f-9fec-c400fdfbd5b3

Now they will:

https://github.com/user-attachments/assets/19b0f022-8e30-412e-8a06-3d9812794219

Author: pulpdrew

.changeset/mean-ghosts-leave.md

@@ -0,0 +1,5 @@
+---
+"@hyperdx/common-utils": patch
+---
+
+fix: Include connectionId in metadata cache key

packages/common-utils/src/__tests__/metadata.test.ts

@@ -198,7 +198,7 @@ describe('Metadata', () => {
 
       // Setup the cache to return the mock data
       mockCache.getOrFetch.mockImplementation((key, queryFn) => {
-        if (key === 'test_db.test_table.metadata') {
+        if (key === 'test_connection.test_db.test_table.metadata') {
           return Promise.resolve(mockTableMetadata);
         }
         return queryFn();
@@ -212,7 +212,7 @@ describe('Metadata', () => {
 
       // Verify the cache was called with the right key
       expect(mockCache.getOrFetch).toHaveBeenCalledWith(
-        'test_db.test_table.metadata',
+        'test_connection.test_db.test_table.metadata',
         expect.any(Function),
       );
 

packages/common-utils/src/metadata.ts

@@ -129,18 +129,21 @@ export class Metadata {
     cache: MetadataCache;
     connectionId: string;
   }) {
-    return cache.getOrFetch(`${database}.${table}.metadata`, async () => {
-      const sql = chSql`SELECT * FROM system.tables where database = ${{ String: database }} AND name = ${{ String: table }}`;
-      const json = await this.clickhouseClient
-        .query<'JSON'>({
-          connectionId,
-          query: sql.sql,
-          query_params: sql.params,
-          clickhouse_settings: this.getClickHouseSettings(),
-        })
-        .then(res => res.json<TableMetadata>());
-      return json.data[0];
-    });
+    return cache.getOrFetch(
+      `${connectionId}.${database}.${table}.metadata`,
+      async () => {
+        const sql = chSql`SELECT * FROM system.tables where database = ${{ String: database }} AND name = ${{ String: table }}`;
+        const json = await this.clickhouseClient
+          .query<'JSON'>({
+            connectionId,
+            query: sql.sql,
+            query_params: sql.params,
+            clickhouse_settings: this.getClickHouseSettings(),
+          })
+          .then(res => res.json<TableMetadata>());
+        return json.data[0];
+      },
+    );
   }
 
   async getColumns({
@@ -153,7 +156,7 @@ export class Metadata {
     connectionId: string;
   }) {
     return this.cache.getOrFetch<ColumnMeta[]>(
-      `${databaseName}.${tableName}.columns`,
+      `${connectionId}.${databaseName}.${tableName}.columns`,
       async () => {
         const sql = chSql`DESCRIBE ${tableExpr({ database: databaseName, table: tableName })}`;
         const columns = await this.clickhouseClient
@@ -240,8 +243,8 @@ export class Metadata {
     metricName?: string;
   }) {
     const cacheKey = metricName
-      ? `${databaseName}.${tableName}.${column}.${metricName}.keys`
-      : `${databaseName}.${tableName}.${column}.keys`;
+      ? `${connectionId}.${databaseName}.${tableName}.${column}.${metricName}.keys`
+      : `${connectionId}.${databaseName}.${tableName}.${column}.keys`;
     const cachedKeys = this.cache.get<string[]>(cacheKey);
 
     if (cachedKeys != null) {
@@ -351,8 +354,8 @@ export class Metadata {
     // HDX-2480 delete line below to reenable json filters
     return []; // Need to disable JSON keys for the time being.
     const cacheKey = metricName
-      ? `${databaseName}.${tableName}.${column}.${metricName}.keys`
-      : `${databaseName}.${tableName}.${column}.keys`;
+      ? `${connectionId}.${databaseName}.${tableName}.${column}.${metricName}.keys`
+      : `${connectionId}.${databaseName}.${tableName}.${column}.keys`;
 
     return this.cache.getOrFetch<{ key: string; chType: string }[]>(
       cacheKey,
@@ -420,9 +423,9 @@ export class Metadata {
     maxValues?: number;
     connectionId: string;
   }) {
-    const cachedValues = this.cache.get<string[]>(
-      `${databaseName}.${tableName}.${column}.${key}.values`,
-    );
+    const cacheKey = `${connectionId}.${databaseName}.${tableName}.${column}.${key}.values`;
+
+    const cachedValues = this.cache.get<string[]>(cacheKey);
 
     if (cachedValues != null) {
       return cachedValues;
@@ -450,28 +453,25 @@ export class Metadata {
       }}
     `;
 
-    return this.cache.getOrFetch<string[]>(
-      `${databaseName}.${tableName}.${column}.${key}.values`,
-      async () => {
-        const values = await this.clickhouseClient
-          .query<'JSON'>({
-            query: sql.sql,
-            query_params: sql.params,
-            connectionId,
-            clickhouse_settings: {
-              max_rows_to_read: String(
-                this.getClickHouseSettings().max_rows_to_read ??
-                  DEFAULT_METADATA_MAX_ROWS_TO_READ,
-              ),
-              read_overflow_mode: 'break',
-              ...this.getClickHouseSettings(),
-            },
-          })
-          .then(res => res.json<Record<string, unknown>>())
-          .then(d => d.data.map(row => row.value as string));
-        return values;
-      },
-    );
+    return this.cache.getOrFetch<string[]>(cacheKey, async () => {
+      const values = await this.clickhouseClient
+        .query<'JSON'>({
+          query: sql.sql,
+          query_params: sql.params,
+          connectionId,
+          clickhouse_settings: {
+            max_rows_to_read: String(
+              this.getClickHouseSettings().max_rows_to_read ??
+                DEFAULT_METADATA_MAX_ROWS_TO_READ,
+            ),
+            read_overflow_mode: 'break',
+            ...this.getClickHouseSettings(),
+          },
+        })
+        .then(res => res.json<Record<string, unknown>>())
+        .then(d => d.data.map(row => row.value as string));
+      return values;
+    });
   }
 
   async getAllFields({
@@ -666,7 +666,7 @@ export class Metadata {
     disableRowLimit?: boolean;
   }) {
     return this.cache.getOrFetch(
-      `${chartConfig.from.databaseName}.${chartConfig.from.tableName}.${keys.join(',')}.${chartConfig.dateRange.toString()}.${disableRowLimit}.values`,
+      `${chartConfig.connection}.${chartConfig.from.databaseName}.${chartConfig.from.tableName}.${keys.join(',')}.${chartConfig.dateRange.toString()}.${disableRowLimit}.values`,
       async () => {
         const selectClause = keys
           .map((k, i) => `groupUniqArray(${limit})(${k}) AS param${i}`)