Skip to content
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

feat: add OTel event creation util func #325

Merged
merged 13 commits into from
Feb 27, 2025

Conversation

bbland1
Copy link
Contributor

@bbland1 bbland1 commented Feb 19, 2025

This PR

  • it should be equivalent to the JS implementation
  • added the logic to return the evaluation flag event

Related Issues

Fixes #322

How to test

  • Unit, Fuzz & E2E tests all pass locally

Signed-off-by: bbland1 <104288486+bbland1@users.noreply.github.com>
Signed-off-by: bbland1 <104288486+bbland1@users.noreply.github.com>
Signed-off-by: bbland1 <104288486+bbland1@users.noreply.github.com>
Signed-off-by: bbland1 <104288486+bbland1@users.noreply.github.com>
Signed-off-by: bbland1 <104288486+bbland1@users.noreply.github.com>
Signed-off-by: bbland1 <104288486+bbland1@users.noreply.github.com>
@bbland1 bbland1 marked this pull request as ready for review February 19, 2025 21:11
@bbland1 bbland1 requested a review from a team as a code owner February 19, 2025 21:11
Copy link

codecov bot commented Feb 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.28%. Comparing base (a55418d) to head (887052d).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #325      +/-   ##
==========================================
+ Coverage   86.78%   88.28%   +1.49%     
==========================================
  Files          13       14       +1     
  Lines        1362     1408      +46     
==========================================
+ Hits         1182     1243      +61     
+ Misses        156      141      -15     
  Partials       24       24              
Flag Coverage Δ
e2e 88.28% <100.00%> (+1.49%) ⬆️
unit 88.28% <100.00%> (+1.49%) ⬆️

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.

beeme1mr and others added 2 commits February 19, 2025 16:27
…e based on codecov report

Signed-off-by: bbland1 <104288486+bbland1@users.noreply.github.com>
Copy link
Member

@beeme1mr beeme1mr left a comment

Choose a reason for hiding this comment

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

This is great, thank you!

…o set what is being set by the if-else

Signed-off-by: bbland1 <104288486+bbland1@users.noreply.github.com>
Copy link
Member

@lukas-reining lukas-reining left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks!

Copy link
Member

@beeme1mr beeme1mr left a comment

Choose a reason for hiding this comment

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

I had a few thoughts after rereviewing this. I'd consider them all optional but worth consider. Thanks!

…dability

Signed-off-by: bbland1 <104288486+bbland1@users.noreply.github.com>
Copy link
Member

@liran2000 liran2000 left a comment

Choose a reason for hiding this comment

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

Some const values were not used, left comments on what to replace.

no linter caught this.

@bbland1
Copy link
Contributor Author

bbland1 commented Feb 26, 2025

@liran2000

Some const values were not used, left comments on what to replace.

no linter caught this.

Nice catch! It took me a moment to see how I missed using the missing const values. Thanks!

Signed-off-by: bbland1 <104288486+bbland1@users.noreply.github.com>
Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

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

Great work!

@toddbaert toddbaert changed the title feat: add Otel util func feat: add OTel event creation util func Feb 27, 2025
@toddbaert toddbaert merged commit 3c70dc2 into open-feature:main Feb 27, 2025
9 checks passed
@bbland1 bbland1 deleted the bbland1/322-add-telemetry-utils branch February 27, 2025 18:34
toddbaert added a commit to open-feature/community that referenced this pull request Feb 27, 2025
@bbland1 has contributed 2 recent changes to the Go SDK:

- open-feature/go-sdk#325
- open-feature/go-sdk#328

@bbland1 when merged, this PR will add you to the org. It comes with no obligation but it's the first step on the https://github.com/open-feature/community/blob/main/CONTRIBUTOR_LADDER.md. Please approve or 👍 this PR to signal your interest!

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add OpenTelemetry-Compatible Telemetry Utility Function
6 participants