feat(openfeature): implement flag evaluation metrics#7993
feat(openfeature): implement flag evaluation metrics#7993dd-oleksii wants to merge 1 commit intomasterfrom
Conversation
Overall package sizeSelf size: 5.5 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.0.1 | 82.56 kB | 817.39 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
|
✨ Fix all issues with BitsAI or with Cursor
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7993 +/- ##
==========================================
- Coverage 73.60% 73.56% -0.04%
==========================================
Files 773 774 +1
Lines 36083 36112 +29
==========================================
+ Hits 26558 26565 +7
- Misses 9525 9547 +22 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
sameerank
left a comment
sameerank
left a comment
There was a problem hiding this comment.
FYI we are currently ignoring this following system test for node: Test_FFE_Eval_Targeting_Key_Optional
At some point we may want to think about making it match the other tracers
Yeah. My understanding is that this should have been fixed in FFL-1730 but it was not |
| if (this.#counter) return this.#counter | ||
|
|
||
| try { | ||
| const { metrics } = require('@opentelemetry/api') |
There was a problem hiding this comment.
This would mean there is now a hard dependency from OpenFeature to OpenTelemetry. Can the API be done in a way that they can be externally combined instead?
There was a problem hiding this comment.
This code is only evaluated when otel is enabled, so it is a soft dependency — not a hard one.
There was a problem hiding this comment.
Heads up. This comment is the last thing blocking the merge and @rochdev is unresponsive on both github and slack.
FFE team's current consensus is that it is a misreading of the code and does not warrant the block. We also use the same approach in Go, Java, Python, and .NET tracers.
I'm dismissing the review to unblock the progress. I'll wait for Monday eve before merging unless I hear any objections 🙏
Add an OpenFeature `finally` hook that emits a `feature_flag.evaluations` OTel counter for every flag evaluation, including error and short-circuit cases. The hook is a no-op when `otelMetricsEnabled` is false.
dd-oleksii
force-pushed
the
oleksii/jj-lwzwlonrxrpr
branch
from
7605a55 to
873af1c
Compare
dd-oleksii
requested review from
greghuels and
leoromanovsky
and removed request for
a team
|
(rebased on latest master to run system-tests) |
BenchmarksBenchmark execution time: 2026-04-16 12:16:52 Comparing candidate commit 873af1c in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 1376 metrics, 108 unstable metrics. |
dd-oleksii
dismissed
rochdev’s stale review
This comment is the last thing blocking the merge and @rochdev is unresponsive on both github and slack.
FFE team's current consensus is that it is a misreading of the code and does not warrant the block. We also use the same approach in Go, Java, Python, and .NET tracers.
I'm dismissing the review to unblock the progress. I'll wait for Monday eve before merging unless I hear any objections 🙏
4 participants
What does this PR do?
Add an OpenFeature
finallyhook that emits afeature_flag.evaluationsOTel counter for every flag evaluation, including error and short-circuit cases. The hook is a no-op whenotelMetricsEnabledis false.Motivation
FFL-1944
Additional Notes
Tested locally against system-tests and it passes except a couple of cases affected by FFL-2064.