Skip to content

feat(CockroachDB support): Switch to JSONField for SDK metrics labels#5833

Merged
khvn26 merged 5 commits intomainfrom
fix/sdk-metrics-labels-jsonb-field
Mar 24, 2026
Merged

feat(CockroachDB support): Switch to JSONField for SDK metrics labels#5833
khvn26 merged 5 commits intomainfrom
fix/sdk-metrics-labels-jsonb-field

Conversation

@khvn26
Copy link
Member

@khvn26 khvn26 commented Jul 25, 2025

Thanks for submitting a PR! Please check the boxes below:

  • I have added information to docs/ if required so people know about the feature!
  • I have filled in the "Changes" section below?
  • I have filled in the "How did you test this code" section below?
  • I have used a Conventional Commit title for this Pull Request

Changes

This migrates all the labels fields added in #5623 to jsonb as the hstore support turned out to be problematic:

  1. The hstore extension requires enabling additional parameter sets in some cloud environments (e.g. Azure).
  2. Psycopg2 type adapters don't play well with CockroachDB, which should be compatible otherwise.

Note that due to problem 1, I've removed the CREATE EXTENSION operation from the original migration.

How did you test this code?

Migrations and tests run ok.
Will be smoke-tested against CockroachDB as well.

@khvn26 khvn26 requested a review from a team as a code owner July 25, 2025 14:33
@khvn26 khvn26 requested review from emyller and removed request for a team July 25, 2025 14:33
@vercel
Copy link

vercel bot commented Jul 25, 2025

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

3 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs Ignored Ignored Preview Mar 24, 2026 1:52pm
flagsmith-frontend-preview Ignored Ignored Preview Mar 24, 2026 1:52pm
flagsmith-frontend-staging Ignored Ignored Preview Mar 24, 2026 1:52pm

Request Review

@github-actions github-actions bot added api Issue related to the REST API feature New feature or request labels Jul 25, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jul 25, 2025

Docker builds report

Image Build Status Security report
ghcr.io/flagsmith/flagsmith-api-test:pr-5833 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-frontend:pr-5833 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-e2e:pr-5833 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith:pr-5833 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-private-cloud:pr-5833 Finished ✅ Results

@github-actions github-actions bot added feature New feature or request and removed feature New feature or request labels Jul 25, 2025
@codecov
Copy link

codecov bot commented Jul 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.32%. Comparing base (7c8e69d) to head (7f0f7a3).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5833   +/-   ##
=======================================
  Coverage   98.32%   98.32%           
=======================================
  Files        1335     1337    +2     
  Lines       49876    49910   +34     
=======================================
+ Hits        49039    49073   +34     
  Misses        837      837           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@scarlson
Copy link
Contributor

Confirming this fix allows CockroachDB to work without needing to update Psycopg to 3.1+.

Copy link
Contributor

@Zaimwa9 Zaimwa9 left a comment

Choose a reason for hiding this comment

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

The migrations look redundant except if I am missing something? Can we see to merge them together maybe ?

@khvn26
Copy link
Member Author

khvn26 commented Jul 28, 2025

@Zaimwa9 Indeed these migrations look very similar, but their purpose is actually different:

  • 0006 adds correct jsonb columns to the tables. The original migration is modified to avoid customers having to add hstore capability to their databases since it's no longer a requirement. This exists primarily for new installations/fresh dbs.
  • 0007 alters existing hstore columns to jsonb. This has to happen for installations where previous 0006 migration was already applied.

So, to answer your question, no, we can't merge these migrations as they cater for different scenarios, and I argue they are not in fact redundant.

@khvn26 khvn26 requested a review from Zaimwa9 July 28, 2025 13:39
Zaimwa9
Zaimwa9 previously approved these changes Jul 28, 2025
Copy link
Contributor

@Zaimwa9 Zaimwa9 left a comment

Choose a reason for hiding this comment

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

Alright thanks for the details, I felt that under the hood there was a migration purpose

@emyller
Copy link
Contributor

emyller commented Jul 28, 2025

I forgot to add as notes to my last review: not actually requesting any changes unless proven necessary by the comment I added.

@github-actions github-actions bot added feature New feature or request and removed feature New feature or request labels Aug 6, 2025
@krahllucas
Copy link

any updates?

@khvn26
Copy link
Member Author

khvn26 commented Mar 13, 2026

@krahllucas Hello, sorry for the radio silence here — we've just prioritised this and are planning to release it until the end of Q1.

khvn26 added 5 commits March 24, 2026 11:48
- Add missing DECLARE for PL/pgSQL loop variable
- Drop column default before type change to avoid cast error
- Renumber migration to 0008 after rebase (0007 conflict)
- Add tests for both fresh install and hstore upgrade paths
@khvn26 khvn26 force-pushed the fix/sdk-metrics-labels-jsonb-field branch from 6c848c6 to 7f0f7a3 Compare March 24, 2026 13:52
@khvn26 khvn26 requested review from Zaimwa9 and emyller March 24, 2026 13:52
@github-actions github-actions bot added feature New feature or request and removed feature New feature or request labels Mar 24, 2026
Copy link
Contributor

@emyller emyller left a comment

Choose a reason for hiding this comment

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

LGTM

@khvn26 khvn26 changed the title feat: Switch to JSONField for SDK metrics labels feat(CockroachDB support): Switch to JSONField for SDK metrics labels Mar 24, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Mar 24, 2026

Playwright Test Results (oss - depot-ubuntu-latest-16)

passed  10 passed

Details

stats  10 tests across 7 suites
duration  24.1 seconds
commit  7f0f7a3
info  🔄 Run: #15446 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-arm-16)

passed  10 passed

Details

stats  10 tests across 7 suites
duration  28.4 seconds
commit  7f0f7a3
info  🔄 Run: #15446 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)

passed  16 passed

Details

stats  16 tests across 13 suites
duration  1 minute, 3 seconds
commit  7f0f7a3
info  🔄 Run: #15446 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  55.6 seconds
commit  7f0f7a3
info  🔄 Run: #15446 (attempt 1)

@khvn26 khvn26 merged commit 02618c6 into main Mar 24, 2026
33 checks passed
@khvn26 khvn26 deleted the fix/sdk-metrics-labels-jsonb-field branch March 24, 2026 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Issue related to the REST API feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants