-
Notifications
You must be signed in to change notification settings - Fork 158
β¨ [FFL-24] add openfeature dependency and datadog provider #3554
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
β¨ [FFL-24] add openfeature dependency and datadog provider #3554
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3554 +/- ##
==========================================
- Coverage 92.34% 92.19% -0.15%
==========================================
Files 319 326 +7
Lines 8109 8237 +128
Branches 1833 1860 +27
==========================================
+ Hits 7488 7594 +106
- Misses 621 643 +22 β View full report in Codecov by Sentry. π New features to boost your workflow:
|
0121d00
to
fa988b8
Compare
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.
So cool to see this starting to be fleshed out! Nice work working through the OpenFeature spec to build out the stubs.
@@ -1,3 +1,27 @@ | |||
# Flagging SDK | |||
|
|||
This package supports flagging and experimentation by performing evaluation in the browser. | |||
|
|||
## Initialize |
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.
β€οΈ
Love the quickstart examples!
packages/flagging/package.json
Outdated
"@openfeature/core": "1.8.0", | ||
"@openfeature/web-sdk": "1.5.0" |
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.
π
import { StandardResolutionReasons, type EvaluationContext, type Logger } from '@openfeature/core' | ||
import { createDatadogProvider } from './provider' | ||
|
||
describe('DatadogProvider', () => { |
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.
π
|
||
describe('resolveBooleanEvaluation', () => { | ||
it('should return default value with DEFAULT reason', () => { | ||
const result = provider.resolveBooleanEvaluation('test-flag', true, mockContext, mockLogger) |
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.
Interesting they want logger
as a fourth argument (package docs). It seems it (and context
) can be set at the provider level. Is this just for extra flexibility?
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.
@aarsilv I don't see a way to set logger on provider level (in provider API). For context, I guess that helps with context switches and it helps for providers in dynamic-context paradigm.
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.
What's even funnier about logger is that OF discourages any logging in resolve methods
import { StandardResolutionReasons } from '@openfeature/core' | ||
import type { Provider } from '@openfeature/web-sdk' | ||
|
||
export function createDatadogProvider(): Provider { |
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.
π
|
||
async function initializeOpenFeature() { | ||
const datadogFlaggingProvider = createDatadogProvider() | ||
await OpenFeature.setContext(subject) |
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.
presumably after this is where we could also call setLogger()
(package docs)
return ( | ||
<div> | ||
<h1>Home</h1> | ||
<h2>Flagging Evaluation</h2> |
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.
π
scripts/check-licenses.js
Outdated
} | ||
printLog('Dependencies check done.') | ||
throw new Error('Dependencies mismatch') |
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.
Thanks for the cleanup and fixing here! π§ π§Ή
c32ec4e
to
d7c9863
Compare
4a6d5e5
to
02ef932
Compare
862769f
to
3078f1b
Compare
3078f1b
to
044fd8c
Compare
@@ -18,13 +18,18 @@ | |||
"@datadog/browser-core": "6.8.0" | |||
}, | |||
"peerDependencies": { | |||
"@datadog/browser-rum": "6.8.0" | |||
"@datadog/browser-rum": "6.8.0", | |||
"@openfeature/web-sdk": "1.5.0" |
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.
question: is this a peer dependency the customer will need to install to use the flagging package?
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.
Yes. Our provider is a plugin for OpenFeature SDK
closing to work on #3571 |
Motivation
Changes
π§ Adds an open feature provider
This will be the basis for datadog flagging customers interacting with the API. It's methods are not yet implemented.
π Fix a bug in the license check file
The output was being reported as:
This should indicate no error but CI was failing.
βοΈ I validated that the script is working by removing entries for open feature from the license file and verifying that the check works:
Test instructions
false
)Checklist