FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: add telemetry helper utils by liran2000 · Pull Request #1346 · open-feature/java-sdk · GitHub

/ java-sdk Public

feat: add telemetry helper utils - #1346

Merged
aepfli merged 7 commits into
open-feature:mainfrom
liran2000:issue/1327
May 13, 2025
ConversationCommits (7)ChecksFiles changed
Merged

feat: add telemetry helper utils#1346
aepfli merged 7 commits into
open-feature:mainfrom
liran2000:issue/1327

Conversation

liran2000 commented Feb 25, 2025

Copy link
Copy Markdown
Member

This PR

adds a method to core that returns a semantically valid flag evaluation event

Related Issues

Fixes #1327

Similar to GO PR.

feat: add telemetry helper utils
5349db8 
Signed-off-by: liran2000 <liran2000@gmail.com>
liran2000 requested a review from a team as a code owner February 25, 2025 08:55
updates
1a33af2 
Signed-off-by: liran2000 <liran2000@gmail.com>

codecov Bot commented Feb 25, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 96.77419% with 1 line in your changes missing coverage. Please review.

Project coverage is 93.35%. Comparing base (58454b4) to head (dbc41b7).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/main/java/dev/openfeature/sdk/Telemetry.java 96.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #1346      +/-   ##
============================================
+ Coverage     92.81%   93.35%   +0.53%     
- Complexity      469      480      +11     
============================================
  Files            43       45       +2     
  Lines          1127     1158      +31     
  Branches         91       99       +8     
============================================
+ Hits           1046     1081      +35     
+ Misses           53       48       -5     
- Partials         28       29       +1
Flag Coverage Δ
unittests 93.35% <96.77%> (+0.53%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

liran2000 requested a review from beeme1mr February 25, 2025 09:56
aepfli approved these changes Feb 25, 2025
View reviewed changes

aepfli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

thank you.

sonarqubecloud Bot commented Feb 27, 2025

Copy link
Copy Markdown

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

chrfwow requested changes Mar 7, 2025
View reviewed changes
Comment thread src/main/java/dev/openfeature/sdk/Telemetry.java Outdated
* @return an EvaluationEvent populated with telemetry data
*/
public static EvaluationEvent createEvaluationEvent(
HookContext<?> hookContext, ProviderEvaluation<?> providerEvaluation) {

chrfwow Mar 7, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Don't we want to use FlagEvaluationDetails here instead of the ProviderEvaluation, as pointed out here?

beeme1mr Mar 7, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yes, good catch. The finally hook was updated in this PR.

beeme1mr previously requested changes Mar 13, 2025
View reviewed changes

beeme1mr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The OTel folks requested that we make a couple of changes in the OTel semcon. Let's wait to merge this until those issues have been resolved.

https://cloud-native.slack.com/archives/C07AES1JN56/p1741799036596509

toddbaert commented Mar 25, 2025

Copy link
Copy Markdown
Member

The OTel folks requested that we make a couple of changes in the OTel semcon. Let's wait to merge this until those issues have been resolved.

https://cloud-native.slack.com/archives/C07AES1JN56/p1741799036596509

@beeme1mr can we revive this PR now?

cc @liran2000

toddbaert requested review from beeme1mr and chrfwow March 25, 2025 16:44

chrfwow commented May 9, 2025

Copy link
Copy Markdown
Contributor

@liran2000 is there any progress on this PR or do you plan to wait for the OTel PR to be merged? I think we can proceed under the assumption that the PR will be merged as it is currently.

liran2000 commented May 9, 2025

Copy link
Copy Markdown
Member Author

@liran2000 is there any progress on this PR or do you plan to wait for the OTel PR to be merged? I think we can proceed under the assumption that the PR will be merged as it is currently.

Hi, waiting for @beeme1mr according to his comment.

fixup: apply changes according to the semconv
2d2104b 
The semconv has changed, and some attributes have been
renamed. Furthermore, the body usage is deprecated
and should be part of the attributes.

see: open-telemetry/semantic-conventions#1990
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
aepfli requested a review from a team as a code owner May 13, 2025 06:34
fixup: fix tests
fe19ca0 
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>

aepfli commented May 13, 2025

Copy link
Copy Markdown
Member

Hey @liran2000, thank you for doing the heavy lifting with this pr. As the semconv has been merged, I adapted your code (renamings mainly) so we can get this merged. Thank you so much!

aepfli dismissed beeme1mr’s stale review May 13, 2025 06:43

the reason for requested changes has been lifted, the semconv is merged, and we could adapt the pull request based on it

aepfli added 2 commits May 13, 2025 08:49
fixup: fix spotless
9d78142 
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
Merge branch 'main' into issue/1327
dbc41b7

sonarqubecloud Bot commented May 13, 2025

Copy link
Copy Markdown

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

aepfli merged commit d0ae548 into open-feature:main May 13, 2025
This was referenced May 13, 2025
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

aepfli aepfli approved these changes
chrfwow chrfwow approved these changes
beeme1mr Awaiting requested review from beeme1mr

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add OpenTelemetry-Compatible Telemetry Utility Function

5 participants

Back | FazBrowse Home | New Git URL