Extract Services out in their own modules so they can be plugged-in as needed

[surbhiia]

Currently, services are part of the bigger core module. It would be beneficial to extract each of them in their own separate modules so they can be plugged-in as needed.

Example use case - Some services like PeriodicWorkService are needed if Disk Buffering is used (to export spans stored in disk to backend). If users/ vendors implement their own disk buffering mechanism, they do not need this service.

There are mainly 4 services (AppLifecycle, VisibleScreenService, CurrentNetworkProvider, PeriodicWorkService) and 2 classes (CacheStorage and Preferences). Objects are created from the 2 classes and the objects can be obtained from the service interface but they're not really services.

• CacheStorage is also only used for disk buffering. I think this can perhaps to moved to the module hosting disk buffering related stuff (under persistence which hosts DiskManager which is the only place it is used right now in).
• Preferences can be provided as a generic utility class maybe via the common module. Right now it is used only in DiskManager for storage and retrieval of MAX_FOLDER_SIZE_KEY),

Following bullets cover what instrumentations rely on what services for more clarity, so we know not all instrumentations require all services

• ANR -> AppLifecycle
• Activity / Fragment Lifecycle -> VisibleScreenService
• NetworkChangeMonitor -> AppLifecycle and CurrentNetworkProvider

[surbhiia]

Currently, services are housed under core -> internal -> services

Changes proposal:

• Preferences can be made a generic utility class instead of being a service. It can be moved from core -> internal -> services -> Preferences.java to core -> Preferences.java or core -> common -> Preferences.java. It's creation can be done directly from the two places from where ServiceManager.initialize is called - OpentelemetryRumBuilder.build() and OpentelemetryRumBuilder when Sdk is Pre Configured (This should probably be moved inside the SdkPreConfiguredRumBuilder class itself). Preferences.java can provide a singleton Preferences object wherever needed.

• CacheStorage: Same as above. It can also be moved to core -> internal -> features -> persistence where DiskManager lives, if we don't perceive it would be required in common module for wider usage.

• ServiceManager : ServiceManger, ServiceManagerImpl can be housed in a new module under root: service - > service-manager. I think we can expose ServiceManger and ServiceManagerImpl as well for the users who want all 3 services. The android-instrumentation module proposed in the linked issue will depend on this as InstallationContext needs the ServiceManager.

• Startable interface: It need to be housed separately to service-manager as otherwise it would lead to a circular dependency - as all services implement the Startable interface and will depend on this module, but if it were housed inside service-manager which also needs to depend on individual services for the getXService() functions, that leads to a circular dependency. May be we can house this under service -> common. ServiceManager also implements the Startable interface

• New modules for all 3 services - AppLifecycle, VisibleScreenService, CurrentNetworkProvider can be housed under service -> app-lifecycle; service -> visible-screen; service -> network-provider.

NOTE - One major problem in the above approach is: All instrumentations need InstallationContext (new android-instrumentation module), that module needs service-manager module due to the 3rd argument in InstallationContext of ServiceManager. And service-manager needs dependence on all services due to the getXService() functions. So, in turn anyway all instrumentations depend on all services. So, I think the solution here could be to define another AutoService interface for these services and then we can load and get services without actual class names in the ServiceManager. Similar style to AndroidInstrumentationLoader - getInstrumentationByType() or getAllInstrumentation() methods.

Services as AutoService:

• Service - would be an AutoService interface. Currently there are two steps to all services - create and start. We can perhaps limit to just start. This interface would have these one/two method declarations. Similar to AndroidInstrumentation interface.
• We would have ServiceLoader and ServiceLoaderImpl similar to current AndroidInstrumentation counterparts.
• Core would depend on the service module with above classes. Core can just load available services. As the added instrumentation modules will have dependency to the relevant services they need. Only exceptions here are - Network Attributes Trace Customization, Screen Attributes Trace Customization, for which we can directly fetch relevant service by type.
• Instrumentations depend on service module as they implement the Service auto-service interface. We do not need to pass in any instance of ServiceManager or ServiceLoader in the InstallationContext. individual instrumentations can retrieve the necessary services by calling ServiceLoader.get().getServiceByTpe("serviceName"). When some user is utilizing just the individual instrumentation without core, they need to start the services in their classpath on their own Or Can we make it a part of the install method code - to start the associated services first, for that we need to ensure that start code is called only once (may be via a alreadyStarted boolean).

Some other classes from core that are used in these services and any other improvements:

• RumConstants - It lives in core -> common folder right now, that can be made a module. or just a common module in root.
• Slow rendering depends on VisibleScreenService just for the DefaultingActivityLifecycleCallbacks class. May be instead of this, directly the super class can be implemented in slow rendering itself.
• I also noticed, some modules depend on common-api module un-necessarily like the slow rendering one.

This is an initial draft. Would be thinking through if it fits all our use cases and refining it more as needed. Do share your feedback on any of it. Also, missed proposal for periodic work service. Would add that as well.

[LikeTheSalad]

A lot of what's mentioned here makes a lot of sense, although before making any final decisions on the matter, I'd like to point out some aspects of "services" to clarify how they are supposed to help us in general, hoping that we can come up with a solution that would still allow us to rely on them for future use cases.

What are Services and why we need them

• Services are helper classes that do either very common tasks or that need to access Android SDK apis. Since Android SDK apis change quite often, and we need to support a wide range of OS versions, having a single place we can make calls to whenever we need to access those apis, such as a service, gives us a lot of flexibility and stability, since we wouldn't have to change many places in our code whenever a new API change is available (because all of them would be done in the service only), while we can still create our own functionality to cover use cases that aren't supported on older OS versions in a way that the rest of our codebase wouldn't need to even know which OS it is running on, reducing the amount of code repetition and probably even device's resources, in case a service makes use of an API that requires storing some value in memory and/or if we can use memoization to reduce the amount of OS API calls needed to get the same result, such as the network connectivity status for example.
• The term "service" might be a bit confusing since it could be interpreted as a long-running process that needs to be started/stopped and that is constantly consuming resources to keep itself running. However, for most of them, that's not the case. They are essentially reusable helper classes, the idea of using the term "service" was to align with Android's "global tools" also called services. You can find more info on those here. In any case, if using "service" provides a wrong idea of them not being lightweight or the like, then I'm open to find a new "grouping term" for them.

Potential issues we should avoid when doing the refactor

Difficult to reuse

Services are meant to be reused, for different reasons which can vary depending on the scope of each service, so I think we need to try and take a look at how we can easily reuse them for future cases.

For example, let's consider PeriodicWorkService. This service is helpful to manage a queue of background work that can be repeated, in a way that reduces the amount of resources that might be needed should we needed to do this from different parts of our agent. For now, it's only used to run periodic exports from the disk by the disk buffering tool, however, most likely in the future we will also require to run periodic network requests to fetch the real time from an NTP server so that distributed tracing works well. If we can reuse PeriodicWorkService for that purpose, not only we wouldn't have to come up with a custom mechanism each time we need to run these kinds of tasks in the background, but also we would be managing the agent's resources better as we can decide from a single place how many secondary threads we would like it to spawn at a time.

For the same use-case of the previous example, we might also need to use the Preferences service, to store the current NTP time and avoid making too many requests to fetch it if it's not needed.

Based on the above, one of my concerns with some of the proposed changes would be that we might move some of the services code, or full classes, to the places where they are needed right now, making it difficult to reuse those tools for future cases.

Easy to overlook

Having the available services in a single place is useful to get an idea of what's available at any time, kind of like having a toolbox when working on a DIY project. I think this can still be accomplished when splitting services into micro-modules, so long as those modules are placed in a single dir. Failing to somehow provide visibility to existing services might cause issues in the future where people start reinventing the wheel in the best case and consuming resources unnecessarily in the worst.

Difficult to scale and maintain

If we decide to place each service in its own micro-module, we might eventually run into a scenario where we get too many modules to maintain, maybe it won't be too much trouble since we use gradle file conventions, though I think it's worth mentioning it just to make sure there's no problem with that, or if maybe we can alternatively group some services if needed to reduce the amount of modules. Also, if it's needed to create a new module just to add a new service I'd be concerned about people deciding to quickly write the code they need in the module they're working on to avoid having to set up a full module, a process that requires creating a set of dirs and files that maybe some people aren't familiarized with. If that ends up happening, then we would lose the benefits of keeping our tools in a single place.

Final thoughts

I think there's a lot of room for improvement for services and I'm up to make changes to them as needed as long as they can still provide the benefits of sharing global tools easily. Probably one way to start could be by trying to see what future usages we might need for them and making sure that those will be easy to accomplish, and for those which we can't think of future usages for now, I think we should come up with a well-documented process of how to make them "globally accessible" if needed in the future.

[surbhiia]

Thanks @LikeTheSalad for your detailed review of the proposal and for explaining the points so clearly! :)

A few take-a-ways and some additional thoughts to carry our discussion forward:

• I think the 1st and easy step (if there isn’t a better approach in the short-term) could be taking services directory out as-is as a separate module under root, core depends on it, all instrumentation depend on it.
• Renaming “service” word and associated terminologies wherever applicable in these. May be reduce the two steps of create and start in these services to just one, if possible?
• Conditional start/initialization of these services based on whether they’re needed or not in core to reduce any unnecessary overload. For example - If I do not enable network change detection instrumentation and don’t need current network attributes in any of my spans, I shouldn’t start the CurrentNetworkProviderService.

Open questions:

• How to strike a balance between too many micro-modules vs having to depend on additional services even if you do not utilize them, while avoiding the issues @LikeTheSalad has mentioned in prior comment?
  • Can dependency injection tools like Dagger/ Hilt help here? I have not worked with those but will look into how they work.

[LikeTheSalad]

The idea I got from the latest SIG meeting was that, as a nice first step, we could extract all services into their own module. Also, we should ensure that services are lazily initialized, preventing unnecessary initialization for environments where they'll never be used. Plus, we should try and keep all the services internal to avoid having to maintain them as a public API.

[surbhiia]

Building on top of what we've discussed so far here and in the sig meeting:

• Keep ServiceManagerImpl as a public API (rename it to perhaps SharedUtilsService / SharedUtilsProvider / CommonUtilsService). This would be the shared utility object provider. Utility objects will be lazily created (we do not need a separate start step) when getXUtil() method is called and stored in a mutable map for later access.
• We don't need ServiceManger interface as we're not exposing these utils as public APIs and don't expect any other implementation of them or the SharedUtilsProvider
• All util packages are in the internal folder in the new module ( perhaps name it shared-utils / common-utils ). SharedUtilsProvider would be the only class outside the internal folder.
• InstallationContext contains a SharedUtilsProvider object. All instrumentations call the relevant getXUtil() method on that object.

Let me know what you all think!

[bidetofevil]

Per what we talked about during the SIG meeting, I summarized my thoughts on this in #702.

tl;dr, I think we should support using the instrumentation independently, but do it in such a way that we don't expose any public API or use the service locator pattern to allow apps to provide their own implementation of services.

[LikeTheSalad]

Thanks @surbhiia! I have a question regarding:

• InstallationContext contains a SharedUtilsProvider object. All instrumentations call the relevant getXUtil() method on that object.

If I understand correctly, since InstallationContext is passed to AndroidInstrumentation.install, and SharedUtilsProvider is the same ServiceManagerImpl but renamed, then it's not clear to me how are we planning to decouple our internal services from the instrumentation API? Since both, AndroidInstrumentation and InstallationContext, are part of the public instrumentation API, any type that they reference is essentially a public API too, which in this case would make SharedUtilsProvider public, along with the "services/utils" it provides.