Skip to content

chore: fix regression from #2912 - tracing in ReadSchema calls#2947

Merged
miparnisari merged 2 commits intomainfrom
fix-tracing-read-schema
Mar 5, 2026
Merged

chore: fix regression from #2912 - tracing in ReadSchema calls#2947
miparnisari merged 2 commits intomainfrom
fix-tracing-read-schema

Conversation

@miparnisari
Copy link
Contributor

@miparnisari miparnisari commented Mar 4, 2026

Description

  • Prevent missing data from trace because of SchemaText() losing the context information
  • add more spans

Before:

before

After:

image

@github-actions github-actions bot added area/schema Affects the Schema Language area/api v1 Affects the v1 API area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) labels Mar 4, 2026
@codecov
Copy link

codecov bot commented Mar 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.54%. Comparing base (d9d89f8) to head (c427f84).
⚠️ Report is 1 commits behind head on main.

❌ Your project status has failed because the head coverage (74.54%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2947      +/-   ##
==========================================
+ Coverage   73.39%   74.54%   +1.16%     
==========================================
  Files         490      490              
  Lines       60376    60385       +9     
==========================================
+ Hits        44306    45008     +702     
+ Misses      12908    12225     -683     
+ Partials     3162     3152      -10     

☔ 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.

@miparnisari miparnisari force-pushed the fix-tracing-read-schema branch from 3084a5a to 52f1815 Compare March 4, 2026 16:32
@miparnisari miparnisari changed the title fix: tracing in SchemaText() chore: fix regression from #2912 - tracing in SchemaText() Mar 4, 2026
@miparnisari miparnisari force-pushed the fix-tracing-read-schema branch from 52f1815 to 299f005 Compare March 4, 2026 22:44
@miparnisari miparnisari force-pushed the fix-tracing-read-schema branch from 299f005 to 1785f40 Compare March 4, 2026 23:14
@miparnisari miparnisari force-pushed the fix-tracing-read-schema branch from 1785f40 to c427f84 Compare March 4, 2026 23:17
@miparnisari miparnisari marked this pull request as ready for review March 4, 2026 23:19
@miparnisari miparnisari requested a review from a team as a code owner March 4, 2026 23:19
@miparnisari miparnisari enabled auto-merge (squash) March 4, 2026 23:25
@miparnisari miparnisari changed the title chore: fix regression from #2912 - tracing in SchemaText() chore: fix regression from #2912 - tracing in ReadSchema calls Mar 4, 2026
// SchemaText returns the schema text at the current revision by reading all namespaces and caveats
// and generating the schema text from them.
func (l *legacySchemaReaderAdapter) SchemaText() (string, error) {
ctx := context.Background()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removing this line is the actual fix

Copy link
Contributor

@tstirrat15 tstirrat15 left a comment

Choose a reason for hiding this comment

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

LGTM


func GenerateSchema(definitions []compiler.SchemaDefinition) (string, bool, error) {
return GenerateSchemaWithCaveatTypeSet(definitions, caveattypes.Default.TypeSet)
return GenerateSchemaWithCaveatTypeSet(context.TODO(), definitions, caveattypes.Default.TypeSet)
Copy link
Contributor

Choose a reason for hiding this comment

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

What does TODO mean? I've never quite understood that. Should it be a .Background()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no idea 😆

@miparnisari miparnisari merged commit 7cf0595 into main Mar 5, 2026
144 of 148 checks passed
@miparnisari miparnisari deleted the fix-tracing-read-schema branch March 5, 2026 03:53
@github-actions github-actions bot locked and limited conversation to collaborators Mar 5, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area/api v1 Affects the v1 API area/schema Affects the Schema Language area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants