Skip to content

Conversation

@pulpdrew
Copy link
Contributor

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:

Screen.Recording.2025-10-15.at.4.26.43.PM.mov

Now they will:

Screen.Recording.2025-10-15.at.4.25.45.PM.mov

@changeset-bot
Copy link

changeset-bot bot commented Oct 15, 2025

🦋 Changeset detected

Latest commit: c6d0eda

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@hyperdx/common-utils Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Oct 15, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
hyperdx-v2-oss-app Ready Ready Preview Comment Oct 16, 2025 3:39pm

@pulpdrew pulpdrew force-pushed the drew/include-connection-in-metadata-cache branch from 3cc0a9f to 0fec8ef Compare October 15, 2025 20:28
@github-actions
Copy link
Contributor

github-actions bot commented Oct 15, 2025

E2E Test Results

All tests passed • 25 passed • 3 skipped • 230s

Status Count
✅ Passed 25
❌ Failed 0
⚠️ Flaky 0
⏭️ Skipped 3

View full report →

@claude
Copy link

claude bot commented Oct 15, 2025

PR Review: Include connectionId in metadata cache key

No critical issues found.

The changes are well-structured and consistently applied across all cache keys in the metadata layer:

What was changed:

  • Updated all metadata cache keys to include connectionId as the first segment
  • Pattern changed from database.table.* to connectionId.database.table.*
  • Applied to: getTableMetadata, getColumns, getMapKeys, getJSONKeys, getKeyValues, and getKeyValues (for chart config)
  • Tests updated to match new cache key format

Quality observations:

  • ✅ Consistent implementation across all cache operations
  • ✅ Tests updated to reflect new cache key format
  • ✅ Changeset properly documented
  • ✅ Solves the stated problem (metadata not shared across connections with same table names)
  • ✅ No security concerns
  • ✅ No breaking API changes

This is a solid bug fix that properly isolates metadata by connection.

@pulpdrew pulpdrew force-pushed the drew/include-connection-in-metadata-cache branch from 0fec8ef to 24e5e1a Compare October 15, 2025 20:30
@pulpdrew pulpdrew requested review from a team and brandon-pereira and removed request for a team October 15, 2025 20:45
Copy link
Contributor

@dhable dhable left a comment

Choose a reason for hiding this comment

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

Looks great

@kodiakhq kodiakhq bot merged commit 3c8f3b5 into main Oct 16, 2025
9 checks passed
@kodiakhq kodiakhq bot deleted the drew/include-connection-in-metadata-cache branch October 16, 2025 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants