Skip to content

feat({ddtrace/opentelemetry,internal/version}): remove Masterminds/semver/v3 #4182

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
dd-mergequeue merged 3 commits into from
Nov 28, 2025
Merged

feat({ddtrace/opentelemetry,internal/version}): remove Masterminds/semver/v3 #4182

dd-mergequeue merged 3 commits into from
Nov 28, 2025

Conversation

@darccio
Copy link
Member

@darccio darccio commented Nov 27, 2025 •
edited
Loading

What does this PR do?

Removes Mastermind/semver/v3 using golang.org/x/mod/semver where appropriate. Custom code to parse a version string has been introduced. Existing tests are passing without modification (only adding new tests).

Stacked on #4181. Please review the following commits:

Motivation

Reducing the scope of our dependencies.

Performance

Old

goos: darwin
goarch: arm64
pkg: github.com/DataDog/dd-trace-go/v2/internal/version
cpu: Apple M1 Max
BenchmarkParseVersion-10    	 2455150	       486.8 ns/op	     467 B/op	       5 allocs/op
PASS
ok  	github.com/DataDog/dd-trace-go/v2/internal/version	1.575s

New

goos: darwin
goarch: arm64
pkg: github.com/DataDog/dd-trace-go/v2/internal/version
cpu: Apple M1 Max
BenchmarkParseVersion-10    	19296534	        55.26 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/DataDog/dd-trace-go/v2/internal/version	1.575s

Reviewer's Checklist

  • Changed code has unit tests for its functionality at or near 100% coverage.
  • System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
  • There is a benchmark for any new code, or changes to existing code.
  • If this interacts with the agent in a new way, a system test has been added.
  • New code is free of linting errors. You can check this by running ./scripts/lint.sh locally.
  • Add an appropriate team label so this PR gets put in the right place for the release notes.
  • Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

@darccio darccio requested review from a team as code owners November 27, 2025 16:10
@github-actions github-actions bot added the apm:ecosystem contrib/* related feature requests or bugs label Nov 27, 2025
@darccio darccio changed the base branch from main to dario.castane/apmlp-494/mapstructure November 27, 2025 16:10
@pr-commenter
Copy link

pr-commenter bot commented Nov 27, 2025 •
edited
Loading

Benchmarks

Benchmark execution time: 2025-11-27 16:46:34

Comparing candidate commit aa0aa13 in PR branch dario.castane/apmlp-494/masterminds-semver with baseline commit 65039e2 in branch dario.castane/apmlp-494/mapstructure.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 3 metrics, 0 unstable metrics.

@darccio darccio removed the request for review from a team November 27, 2025 16:14
@darccio darccio changed the title feat(internal/version): remove Masterminds/semver/v3 feat({ddtrace/opentelemetry,internal/version}): remove Masterminds/semver/v3 Nov 27, 2025
@darccio darccio mentioned this pull request Nov 27, 2025
Closed
7 tasks
genesor
genesor reviewed Nov 27, 2025
View reviewed changes
ddtrace/opentelemetry/otel_test.go

func expectedSpanNames() []string {
v := semver.MustParse(otelhttp.Version())
if v.Compare(semver.MustParse("0.60.0")) <= 0 {
Copy link
Member

@genesor genesor Nov 27, 2025

Choose a reason for hiding this comment

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

Why was this needed before and not anymore ?

In the go.mod of the tracer we have a dependency on v0.49.0

Copy link
Member Author

@darccio darccio Nov 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This was a temporary solution. It was done in #3551.

I realize that I didn't check the otelhttp version, but apparently... The smoke tests are passing. I think they did some significant back-and-forth changes between v0.61.0 and v0.63.0:

v0.61.0 (link)

Remove support for the OTEL_SEMCONV_STABILITY_OPT_IN environment variable as well as support for semantic conventions v1.20.0 in the modules below. (open-telemetry/opentelemetry-go-contrib#7584)
go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful
go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin
go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux
go.opentelemetry.io/contrib/instrumentation/github.com/labstack/echo/otelecho
go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp

v0.62.0 (link)

The semantic conventions have been upgraded from v1.26.0 to v1.34.0 in go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp. (open-telemetry/opentelemetry-go-contrib#7383, open-telemetry/opentelemetry-go-contrib#7484)

v0.63.0 (link)

Switched the default for OTEL_SEMCONV_STABILITY_OPT_IN to emit the v1.26.0 semantic conventions by default in the following modules.

go.opentelemetry.io/contrib/instrumentation/github.com/emicklei/go-restful/otelrestful
go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin
go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux
go.opentelemetry.io/contrib/instrumentation/github.com/labstack/echo/otelecho
go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp
The OTEL_SEMCONV_STABILITY_OPT_IN=http/dup environment variable can be still used to emit both the v1.20.0 and v1.26.0 semantic conventions.
It is however impossible to emit only the 1.20.0 semantic conventions, as the next release will drop support for that environment variable. (open-telemetry/opentelemetry-go-contrib#6899)

@darccio darccio mentioned this pull request Nov 27, 2025
Merged
7 tasks
@darccio
Copy link
Member Author

darccio commented Nov 28, 2025

/merge

@dd-devflow-routing-codex
Copy link

dd-devflow-routing-codex bot commented Nov 28, 2025 •
edited
Loading

View all feedbacks in Devflow UI.

2025-11-28 14:34:54 UTC ℹ️ Start processing command /merge

2025-11-28 14:34:57 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in dario.castane/apmlp-494/mapstructure is approximately 0s (p90).

2025-11-28 15:25:27 UTC ℹ️ MergeQueue: This merge request was merged

@dd-mergequeue dd-mergequeue bot merged commit 20a6618 into dario.castane/apmlp-494/mapstructure Nov 28, 2025
273 checks passed
@dd-mergequeue dd-mergequeue bot deleted the dario.castane/apmlp-494/masterminds-semver branch November 28, 2025 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@genesor genesor genesor approved these changes

Assignees

No one assigned

Labels

apm:ecosystem contrib/* related feature requests or bugs mergequeue-status: done

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@darccio @genesor