feat(ddtrace): implement version detection for dd-trace-go v1 non-transitional#3381
Conversation
Datadog ReportBranch report: ✅ 0 Failed, 4475 Passed, 64 Skipped, 3m 40.55s Total Time |
| minor, _ := strconv.Atoi(captures["mi"]) | ||
| patch, _ := strconv.Atoi(captures["pa"]) | ||
| rc, _ := strconv.Atoi(captures["rc"]) | ||
| return major, minor, patch, rc |
There was a problem hiding this comment.
Is this an unrelated refactoring? Or was the plan to use parseVersion in v1.go? If yes, why didn't this pan out?
There was a problem hiding this comment.
@felixge I pushed and left behind a change I wanted to introduce to replace the Tag with the v1 transitional version, so logs show the version that users can see in their go.mod as direct dependency (to avoid confusions).
You can see the change at 31564a3.
I'm also wondering if it's the right time to replace this ad hoc code with semver instead. WDYT?
Update: I woke up with an idea about how to refactor it so we check debuginfo once, and also warn on tracer's start: 59b5ceb
There was a problem hiding this comment.
@felixge I rebased this against main. Please, can you review it again? Thanks!
There was a problem hiding this comment.
@kakkoyun PR is going to have lots of changed go.mod files due to adding a new dependency 😁 But worth it, it looks cleaner.
BenchmarksBenchmark execution time: 2025-04-10 08:47:54 Comparing candidate commit ef705e9 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 55 metrics, 1 unstable metrics. |
5b36869 to
63b79ab
Compare
| minor, _ := strconv.Atoi(captures["mi"]) | ||
| patch, _ := strconv.Atoi(captures["pa"]) | ||
| rc, _ := strconv.Atoi(captures["rc"]) | ||
| return major, minor, patch, rc |
|
|
||
| func parseVersion(version string) (int, int, int, int) { | ||
| var ( | ||
| v = semver.MustParse(version) |
There was a problem hiding this comment.
I decided to go with MustParse because this shouldn't fail ever. I consider this a canary.
rarguelloF
left a comment
There was a problem hiding this comment.
lgtm, left a refactor suggestion
|
Nice:
The |
a1c8934 to
757ae2d
Compare
…nsitional Co-authored-by: Kemal Akkoyun <kakkoyun@users.noreply.github.com>
757ae2d to
ef705e9
Compare
## Summary of changes
Adds flag evaluation metrics support to the Datadog OpenFeature
provider. When a feature flag is evaluated, a counter metric is emitted
with relevant attributes.
## Reason for change
Part of the Feature Flags Evaluation (FFE) initiative to provide
visibility into flag evaluations across all Datadog server SDKs. This
brings parity with implementations in dd-trace-py and dd-trace-go.
## Implementation details
- **FlagEvalMetrics.cs**: Core metrics recording class using
`System.Diagnostics.Metrics`
- Meter name: `Datadog.FeatureFlags.OpenFeature`
- Metric: `feature_flag.evaluations` (Counter<long>)
- Unit: `{evaluation}`
- Tags: `feature_flag.key`, `feature_flag.result.variant`,
`feature_flag.result.reason`, `error.type` (optional),
`feature_flag.result.allocation_key` (optional)
- **FlagEvalMetricsHook.cs**: OpenFeature hook that implements
`FinallyAsync` to record metrics after each evaluation completes
- Converts reason to lowercase for consistency
- Maps `ErrorType` enum to snake_case strings
- Extracts allocation key from flag metadata when present
- **DatadogProvider.cs**: Registers the metrics hook with the
OpenFeature API
- Updated OpenFeature SDK dependency from 2.0.0 to 2.3.0
- **Package version bumped to 2.3.0** to match OpenFeature SDK
dependency version. This signals the breaking changes from OpenFeature
2.3.0:
- .NET 6 support dropped (now requires .NET 8+)
- Hook `finally` signature changed to include evaluation details
**Conditional compilation**: Only enabled for .NET 6+ (`#if
NET6_0_OR_GREATER`) since `System.Diagnostics.Metrics` requires .NET 6+.
## Test coverage
- Feature is validated through system-tests
(`tests/ffe/test_flag_eval_metrics.py`)
- Also checked in DataDog/ffe-dogfooding#56
- Unit tests were explored but deferred due to type conflicts between
shared source files in `Datadog.FeatureFlags.OpenFeature` and
`Datadog.Trace` assemblies
## Other details
Jira: https://datadoghq.atlassian.net/browse/FFL-1946
Related PRs:
- dd-trace-py: DataDog/dd-trace-py#17029
- dd-trace-go: DataDog/dd-trace-go#3381
- system-tests: DataDog/system-tests#6689
---------
Co-authored-by: Andrew Lock <andrew.lock@datadoghq.com>
What does this PR do?
Adds v1 non-transitional versions of dd-trace-go present in the compiled binary.
It also sets the v1 transitional version as the tag version so logging for services instrumented with
v1.74.0and higher show a v1 version, following the principle of least surprise.Motivation
v1.73.0 and below are not compatible with v2. They can't coexist.
Reviewer's Checklist
v2-devbranch and reviewed by @DataDog/apm-go.Unsure? Have a question? Request a review!