Skip to content

Preserve text glob patterns during export#2510

Merged
mmat11 merged 1 commit into
open-telemetry:mainfrom
MrAlias:fix-v2-conf-export
Jun 26, 2026
Merged

Preserve text glob patterns during export#2510
mmat11 merged 1 commit into
open-telemetry:mainfrom
MrAlias:fix-v2-conf-export

Conversation

@MrAlias

@MrAlias MrAlias commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Motivation

  • Runtime v2 export serialized glob selectors via GlobAttr.MarshalYAML() but GlobAttr.UnmarshalText() (used for env/text-backed values) compiled the pattern without preserving its original text, causing exported rules to lose their glob strings.
  • The change ensures env/text-provided glob selectors round-trip through marshal/unmarshal so exported v2 configs remain equivalent to the original runtime selection criteria.

Description

  • Populate the internal str field in GlobAttr.UnmarshalText and clear it for empty input so that MarshalYAML returns the original pattern for env/text-loaded globs (pkg/appolly/services/attr_glob.go).
  • Add a unit test verifying that UnmarshalText preserves the pattern for subsequent MarshalYAML and that matching still works (pkg/appolly/services/attr_glob_test.go).
  • Add an integration-style unit case to internal/config/convert/export_test.go asserting that env-backed top-level AutoTargetExe and AutoTargetLanguage selectors are exported with exe_path_glob and language_glob set correctly.

Testing

  • go test ./pkg/appolly/services -run TestGlobAttrUnmarshalTextPreservesPatternForMarshalYAML -count=1 -v
  • go test ./pkg/appolly/services -count=1
  • go test ./internal/config/convert -run TestRuntimeToV2EffectiveDiscoveryCriteria -count=1 -v

@MrAlias MrAlias added this to the v0.10.0 milestone Jun 26, 2026
@MrAlias MrAlias requested a review from a team as a code owner June 26, 2026 00:19
@MrAlias MrAlias added bug Something isn't working go Related to Go code labels Jun 26, 2026
@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 68.95%. Comparing base (5f46b4d) to head (a3c4e70).

Files with missing lines Patch % Lines
pkg/appolly/services/attr_glob.go 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2510      +/-   ##
==========================================
- Coverage   68.99%   68.95%   -0.04%     
==========================================
  Files         345      345              
  Lines       46740    46743       +3     
==========================================
- Hits        32247    32233      -14     
- Misses      12451    12466      +15     
- Partials     2042     2044       +2     
Flag Coverage Δ
integration-test 50.88% <60.00%> (+0.24%) ⬆️
integration-test-arm 26.94% <0.00%> (-0.14%) ⬇️
integration-test-vm-5.15-lts ?
integration-test-vm-6.18-lts ?
k8s-integration-test 36.14% <60.00%> (+0.40%) ⬆️
oats-test 35.31% <0.00%> (+0.03%) ⬆️
unittests 63.31% <80.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.

@mariomac mariomac left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch!

@mmat11 mmat11 merged commit 914ffa4 into open-telemetry:main Jun 26, 2026
98 checks passed
@MrAlias MrAlias deleted the fix-v2-conf-export branch June 26, 2026 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working go Related to Go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants