Hide symbols from public api #1278
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1278 +/- ##
==========================================
+ Coverage 64.30% 64.32% +0.01%
==========================================
Files 142 142
Lines 3012 3013 +1
Branches 296 296
==========================================
+ Hits 1937 1938 +1
Misses 998 998
Partials 77 77 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| private var state = State.FOREGROUND | ||
|
|
||
| // for testing | ||
| @OptIn(Incubating::class) |
There was a problem hiding this comment.
Is this needed, given that the constructor is internal?
There was a problem hiding this comment.
Yes - @Incubating propagates so that any usage of an annotated symbol will require an opt-in, regardless of visibility. This suppresses a couple of compiler warnings that got introduced in the PR that added it. The alternative is to remove the annotation from SessionConfig
| import java.util.Random | ||
|
|
||
| interface SessionIdGenerator { | ||
| internal fun interface SessionIdGenerator { |
There was a problem hiding this comment.
Oh I think this was public on purpose. There have been users who previously wanted to provide their own session id values....
There was a problem hiding this comment.
I don't see any public-facing API that allows setting a custom SessionIdGenerator or SessionStorage - what's the expected usage?
There was a problem hiding this comment.
It's not currently something users can provide via the initializer. The options we have are either hide them until needed, as proposed with these changes, or expose them right away, maybe via the SessionConfig object or the like. I'm fine either way tbh, though if we expose them right away, it's probably good to mark them all as incubating. Wdyt, @breedx-splk?
| import io.opentelemetry.android.session.Session | ||
|
|
||
| interface SessionStorage { | ||
| internal interface SessionStorage { |
There was a problem hiding this comment.
Same comment -- I think this was visible because some users wanted to be able to provide their own storage mechanism?
Goal
This changeset hides symbol from the public API exposed in
agent-api.SessionStorageandSessionIdGeneratordon't appear to be usable for an external consumer of the library, so IMO it makes sense to hide the concepts for now. I additionally separated out the implementations from the interfaces.