-
Notifications
You must be signed in to change notification settings - Fork 485
feat(openfeature): add Datadog OpenFeature provider for server-side feature flag evaluation #4056
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.
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
Conversation
BenchmarksBenchmark execution time: 2025-10-27 13:21:22 Comparing candidate commit f9ae5bf in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 5 metrics, 0 unstable metrics. scenario:BenchmarkStartSpanConfig/scenario_WithStartSpanConfig-24
|
Automatied benchmarks is really interested - will we be able to wire in the flag evaluations into it to see if changes cause regressions too? I took a pass at doing this in the eppo repository for memory allocations, which are a customer sensitivity - we'd like to operate with a light touch in our customers' environments: https://github.com/Eppo-exp/golang-sdk/blob/main/.github/workflows/memory-profile-compare.yml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very strong foundation!
|
ff5303e to
32d50dc
Compare
4906534 to
71cd401
Compare
@leoromanovsky I added benchmarks and registered them into the continuous benchmarking platform. This will take a while for them to show up because it needs to gather a lot of data points by running on main |
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Init initializes the provider. For the Datadog provider, | ||
| // this is waiting for the first configuration to be loaded. | ||
| func (p *DatadogProvider) Init(openfeature.EvaluationContext) error { | ||
| p.mu.Lock() | ||
| defer p.mu.Unlock() | ||
| for p.configuration == nil { | ||
| p.configChange.Wait() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid blocking forever when no configuration arrives
The provider’s Init method waits on the condition variable until p.configuration becomes non‑nil and never returns otherwise. If Remote Config is unavailable, misconfigured, or simply has no flags for this environment, this wait loop never exits and the OpenFeature client never finishes initialization. In practice this can hang application startup indefinitely even though evaluations could proceed with default values. Consider adding a timeout or returning immediately when Remote Config is unavailable so the provider can be registered and errors can be reported per evaluation instead of deadlocking initialization.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leoromanovsky Is this a known issue of the openfeature.StateHandler interface not giving us a way to timeout ? I am against putting a constant timeout to this waiting time because remoteconfig api contract has no guaranteed init time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not known - when you encounter something like that which you feel we could improve OF upstream with could you please add a ticket to (https://datadoghq.atlassian.net/browse/FFL-1147); we will follow up with pull requests.
| // Special handling for "id" attribute: if not explicitly provided, use targeting key | ||
| if condition.Attribute == "id" && !exists { | ||
| if targetingKey, ok := context[of.TargetingKey].(string); ok { | ||
| attributeValue = targetingKey | ||
| exists = true | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no special handling is desired here; callers must provide the targeting key
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this to make one of the system-tests pass 🤔 @khanayan123 What do you think ?
| // Returns an error if the default configuration of the Remote Config client is NOT working | ||
| // In this case, please call tracer.Start before creating the provider. | ||
| func NewDatadogProvider() (*DatadogProvider, error) { | ||
| if !internal.BoolEnv(ffeProductEnvVar, false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leoromanovsky, are we sure about this activation env var? Please think of the customer implications and experience. IMHO, this is useful for emergency disablement for ops teams (ie. without touching the source code), and that can be achieved in other ways too.
Signed-off-by: Eliott Bouhana <[email protected]>
Signed-off-by: Eliott Bouhana <[email protected]>
Signed-off-by: Eliott Bouhana <[email protected]>
Signed-off-by: Eliott Bouhana <[email protected]>
Signed-off-by: Eliott Bouhana <[email protected]>
Signed-off-by: Eliott Bouhana <[email protected]>
Signed-off-by: Eliott Bouhana <[email protected]>
Signed-off-by: Eliott Bouhana <[email protected]>
98754ac to
c9e1b6c
Compare
|
/merge |
|
View all feedbacks in Devflow UI.
This merge request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
The expected merge time in
|
Signed-off-by: Eliott Bouhana <[email protected]>
What does this PR do?
Motivation
This PR adds the Datadog OpenFeature provider to dd-trace-go, enabling Go applications to evaluate feature flags using the OpenFeature standard
interface.
Testing Instructions
System Tests run and DataDog/system-tests#5580
Benchmarks