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

Changing access control level for few methods in SpanAdapter #479

Closed
wants to merge 3 commits into from

Conversation

vj4iosdev
Copy link
Contributor

What's changed in this PR?

  • Changing the access control level for the below methods from internal to public in SpanAdapter
  1. toProtoSpanKind()
  2. toProtoSpanEvent()
  3. toProtoSpanLink()
  4. toStatusProto()
  • One minor change in OtlpTraceJsonExporter i,e changing the access control for the method getExportedSpans() to public from internal. I had contributed to this earlier but we were using this file locally. Now we are upgrading to use this directly from this repo.

Why is this change required?

  • We are leveraging OpenTelemetry extensively for both mobile and web in our app (nested as well).
    In some of the flows, the WebView generates the Spans and hands over the SpanData to mobile. In this case, the native side, instead of creating new Spans/ids, needs to override the SpanData with the received SpanData (containing already generated span ids.). So we need the above-mentioned methods to be public to do so. Hence opening the PR with the changes required.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 25, 2023

CLA Missing ID CLA Not Signed

@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Files Coverage Δ
...penTelemetryProtocolCommon/trace/SpanAdapter.swift 99.00% <100.00%> (ø)
...etryProtocolGrpc/trace/OtlpTraceJsonExporter.swift 88.57% <100.00%> (ø)

... and 1 file with indirect coverage changes

📢 Thoughts on this report? Let us know!.

@vj4iosdev
Copy link
Contributor Author

Closing this PR and will open another one due to EasyCLA missing ID commit issue.

@vj4iosdev vj4iosdev closed this Oct 26, 2023
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.

1 participant