-
Notifications
You must be signed in to change notification settings - Fork 158
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 #480
Changing access control level for few methods in SpanAdapter #480
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
... and 2 files with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
@nachoBonafonte @bryce-b @vvydier Please take a look whenever you get some time. |
It looks good to me, I completely agree with the |
Ok. So here is our use case. When we receive spanData from web FCI to mobile FCI, we are required not to generate the spanID, traceID, parentSpanID and span attributes but override these as received from the web. So here is our code:
And in the |
Thanks for the clarification it makes total sense now. If you think that making span adapter a class with the open method works better for you, fellow free to change it. |
Thanks for the approval @nachoBonafonte . |
What's changed in this PR?
internal
topublic
inSpanAdapter
OtlpTraceJsonExporter
i,e changing the access control for the methodgetExportedSpans()
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?
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.