Skip to content

fix(setup): warn when instrumentation hooks bump consumer versions#502

Merged
y1yang0 merged 5 commits into
open-telemetry:mainfrom
gyanranjanpanda:fix/silent-version-bump-489
May 18, 2026
Merged

fix(setup): warn when instrumentation hooks bump consumer versions#502
y1yang0 merged 5 commits into
open-telemetry:mainfrom
gyanranjanpanda:fix/silent-version-bump-489

Conversation

@gyanranjanpanda
Copy link
Copy Markdown
Contributor

Problem

The syncDeps function silently bumps the consumer's go directive and OTel dependency versions during setup. Because go.mod is restored by cleanup after a successful build, the user never notices these version changes happened.

Solution

This implements Part B of the proposed fix from #489: before running go mod tidy, we snapshot the consumer's go directive and direct dependency versions. After tidy, we compare and emit Warn-level log messages for every version that was raised.

Example output

WARN go directive bumped by instrumentation hooks from=1.22.0 to=1.25.0
WARN dependency version bumped by instrumentation hooks module=go.opentelemetry.io/otel from=v1.38.0 to=v1.43.0

Changes

tool/internal/setup/sync.go

Symbol Purpose
versionSnapshot Struct holding goVersion and deps map[string]string
snapshotVersions() Captures pre-mutation state from the parsed modfile.File
warnVersionBumps() Re-parses go.mod post-tidy, compares against snapshot, emits warnings

Design decisions:

  • Go directives use bare versions (1.25.0) while semver.Compare needs a v prefix -- we prepend v before comparison
    • Only direct (non-indirect) dependencies are tracked to avoid noise
    • warnVersionBumps is non-fatal: parse errors are logged as warnings, not returned as errors
    • Uses semver.IsValid guards to skip malformed versions gracefully

tool/internal/setup/sync_test.go

6 new tests covering:

  • TestSnapshotVersions -- direct deps captured, indirect excluded
    • TestSnapshotVersions_MinimalGoMod -- empty deps, go version captured
    • TestWarnVersionBumps_GoVersionRaised -- go directive bump produces warning
    • TestWarnVersionBumps_DepVersionRaised -- OTel dep bump produces warning
    • TestWarnVersionBumps_NoChange -- no warnings when versions match
    • TestWarnVersionBumps_MissingFile -- graceful handling of missing go.mod

What's NOT in this PR

Part A (lowering hook module floors from go 1.25.0 / otel v1.43.0 to true minimums) is a separate effort requiring CI matrix testing across Go versions.

Fixes #489

@gyanranjanpanda gyanranjanpanda requested a review from a team as a code owner May 13, 2026 21:17
@github-actions github-actions Bot added the scope:fix A bug that is being fixed label May 13, 2026
@gyanranjanpanda gyanranjanpanda force-pushed the fix/silent-version-bump-489 branch 2 times, most recently from 2293928 to d9495bd Compare May 13, 2026 21:46
syncDeps was silently raising the consumer go directive and OTel dep
versions via go mod tidy. Snapshot the go.mod before mutation and diff
after tidy to surface what changed and which hooks caused it.

Fixes open-telemetry#489
@gyanranjanpanda gyanranjanpanda force-pushed the fix/silent-version-bump-489 branch from d9495bd to 889e511 Compare May 13, 2026 21:51
@gyanranjanpanda
Copy link
Copy Markdown
Contributor Author

could @txabman42 review this

Comment thread tool/internal/setup/sync.go Outdated
Comment thread tool/internal/setup/sync.go Outdated
Comment thread tool/internal/setup/sync.go Outdated
Comment thread tool/internal/setup/sync.go Outdated
Comment thread tool/internal/setup/sync.go Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 14, 2026

Codecov Report

❌ Patch coverage is 92.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.00%. Comparing base (e88e6f5) to head (b308ae3).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
tool/internal/setup/sync.go 92.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           main     #502       +/-   ##
=========================================
+ Coverage      0   63.00%   +63.00%     
=========================================
  Files         0       62       +62     
  Lines         0     4790     +4790     
=========================================
+ Hits          0     3018     +3018     
- Misses        0     1527     +1527     
- Partials      0      245      +245     
Flag Coverage Δ
tool 63.00% <92.00%> (?)

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

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds warning logs during setup when instrumentation hook dependency synchronization raises a consumer module’s Go directive or direct dependency versions after go mod tidy.

Changes:

  • Adds version snapshotting before go mod tidy.
  • Adds post-tidy comparison and warning logs for raised Go/dependency versions.
  • Adds unit tests for snapshotting and warning behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tool/internal/setup/sync.go Adds snapshot/compare logic and emits warnings after dependency sync.
tool/internal/setup/sync_test.go Adds tests for snapshot capture and warning scenarios.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tool/internal/setup/sync.go Outdated
@gyanranjanpanda
Copy link
Copy Markdown
Contributor Author

gyanranjanpanda commented May 14, 2026

i adressed all issue @y1yang0 u mention could u verify this if any thing i can change i am willing to do

Comment thread tool/internal/setup/sync.go Outdated
Comment thread tool/internal/setup/sync.go Outdated
Copy link
Copy Markdown
Contributor

@txabman42 txabman42 left a comment

Choose a reason for hiding this comment

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

LGTM once @y1yang0 comments are resolved!

@gyanranjanpanda
Copy link
Copy Markdown
Contributor Author

gyanranjanpanda commented May 15, 2026

i adressd the latest @y1yang0 issue could u review this

Comment thread tool/internal/setup/sync.go
Comment thread tool/internal/setup/sync.go
Comment thread tool/internal/setup/sync_test.go
@gyanranjanpanda
Copy link
Copy Markdown
Contributor Author

Thanks, fixed. I removed the repeated standalone Go language-version test and folded it into TestWarnVersion_GoVersionRaised as a table-driven case. This keeps coverage for both 1.22.0 -> 1.25.0 and 1.21 -> 1.25.0 while avoiding duplicate setup/assertions.

Verified with:

go test ./tool/internal/setup
could @amazingakai  verify now

@y1yang0 y1yang0 merged commit ac28fff into open-telemetry:main May 18, 2026
45 checks passed
@gyanranjanpanda gyanranjanpanda deleted the fix/silent-version-bump-489 branch May 18, 2026 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope:fix A bug that is being fixed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

setup: silently bumps consumer's go version and OTel dependencies

5 participants