-
Notifications
You must be signed in to change notification settings - Fork 38
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
Restructure modules - phase 3 #276
Restructure modules - phase 3 #276
Conversation
@@ -370,7 +372,9 @@ private void applyConfiguration() { | |||
if (initializationEvents == InitializationEvents.NO_OP) { | |||
initializationEvents = new SdkInitializationEvents(); | |||
} | |||
initializationEvents.recordConfiguration(config); | |||
Map<String, String> configMap = new HashMap<>(); | |||
// TODO: Convert config to map |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened #277 to track this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wish there could be a way to pass the config object without having to transform it into a map though, it feels awkward. But I guess we can leave it as is for now. I'll follow up on the ticket on this.
...rc/test/java/io/opentelemetry/android/instrumentation/activity/VisibleScreenTrackerTest.java
Outdated
Show resolved
Hide resolved
...tup/src/main/java/io/opentelemetry/android/instrumentation/startup/InitializationEvents.java
Show resolved
Hide resolved
android-agent/build.gradle.kts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we should add the fragment module too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's used indirectly through the Activity instrumentation. If we want to change that, can we do it in a separate PR plz?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah got it, that's fine then, I thought it was not being added. Cheers!
This is the continuation of #267 and #269.
This migrates the subdirs in instrumentation to be their own instrumentation modules, as discussed previously.