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 OpenTelemetry Support #198

Merged
merged 18 commits into from
Mar 11, 2025
Merged

feat: Add OpenTelemetry Support #198

merged 18 commits into from
Mar 11, 2025

Conversation

Abhi347
Copy link
Member

@Abhi347 Abhi347 commented Sep 17, 2024

feat: Add OpenTelemetry Support
feat: Add Support for Tracing Propagation Headers

Things to consider:

  • The current PR might introduce a breaking change since the OpenCensus java files have new package. To mitigate this, I have kept the original classes at their place and extended them from the new classes. Additionally, marked them as deprecated too. This should resolve any breaking change for existing users of the tracing.

Copy link

codecov bot commented Sep 17, 2024

Codecov Report

Attention: Patch coverage is 74.82993% with 111 lines in your changes missing coverage. Please review.

Project coverage is 76.89%. Comparing base (8ddc6d0) to head (331caad).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...va/com/spotify/github/v3/clients/GitHubClient.java 75.08% 63 Missing and 7 partials ⚠️
...fy/github/tracing/opencensus/OpenCensusTracer.java 43.24% 21 Missing ⚠️
...java/com/spotify/github/v3/clients/NoopTracer.java 65.00% 7 Missing ⚠️
...n/java/com/spotify/github/tracing/TraceHelper.java 78.57% 3 Missing ⚠️
...hub/tracing/opentelemetry/OpenTelemetryTracer.java 91.17% 2 Missing and 1 partial ⚠️
...in/java/com/spotify/github/tracing/BaseTracer.java 83.33% 2 Missing ⚠️
...tify/github/tracing/opencensus/OpenCensusSpan.java 90.00% 2 Missing ⚠️
...ithub/tracing/opentelemetry/OpenTelemetrySpan.java 90.00% 2 Missing ⚠️
...om/spotify/github/opencensus/OpenCensusTracer.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #198      +/-   ##
============================================
- Coverage     77.71%   76.89%   -0.83%     
- Complexity      349      381      +32     
============================================
  Files            50       56       +6     
  Lines          1149     1277     +128     
  Branches         46       51       +5     
============================================
+ Hits            893      982      +89     
- Misses          226      264      +38     
- Partials         30       31       +1     

☔ 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.

@Abhi347 Abhi347 force-pushed the feat/add-ot-support branch 2 times, most recently from cfd6d53 to d5b90e3 Compare September 17, 2024 15:36

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
feat: Add OpenTelemetry Support
feat: Add Support for Tracing Propagation Headers
@Abhi347 Abhi347 force-pushed the feat/add-ot-support branch from d5b90e3 to 92c9da5 Compare September 23, 2024 09:00
Abhi347 and others added 6 commits January 14, 2025 14:35
@Abhi347 Abhi347 marked this pull request as ready for review January 29, 2025 12:41
Abhi347 and others added 6 commits January 29, 2025 15:19
…n and Tracer
@Abhi347 Abhi347 force-pushed the feat/add-ot-support branch from 9a24885 to 1c76cd0 Compare March 10, 2025 11:00
Abhi347 and others added 4 commits March 10, 2025 13:30
@dennisgranath
Copy link
Contributor

It's a bit unclear from you description whether this is a breaking change or not? I guess we don't want any breaking changes but add opentelemetry support? Is that what you mean by this?

@Abhi347
Copy link
Member Author

Abhi347 commented Mar 10, 2025

It's a bit unclear from you description whether this is a breaking change or not? I guess we don't want any breaking changes but add opentelemetry support? Is that what you mean by this?

@dennisgranath It's not breaking change anymore since I have added the duplicated classes at their previous position and marked them as deprecated. I'll verify tomorrow if this is still going to cause some issues. Updating the description accordingly.

Update: Confirmed that it's not a breaking change.

@Abhi347 Abhi347 merged commit 60b86bd into master Mar 11, 2025
3 of 5 checks passed
@Abhi347 Abhi347 deleted the feat/add-ot-support branch March 11, 2025 12:55
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.

None yet

2 participants