open-telemetry / open-telemetry/opentelemetry-android
Refactoring suggestions for network attributes related stuff
Nobody has claimed this yet.
- Dominant language
- Kotlin
- Stars
- 297
- Forks
- 113
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 51
Description
Currently, the project has the following modules/folders related to network attributes:
- Network Change Instrumentation Module - Hosts files related to the instrumentation itself (instrumenter creation, attribute extraction, registering network change listener in CurrentNetworkProvider service etc)
- CurrentNetworkProvider Service (folder in services module)- Hosts files related to the service that keeps track of the CurrentNetwork and notifies all listeners onNetworkchange.
- Network data model and CurrentNetworkAttributesExtractor (folder in common module)
- NetworkAttributesSpanAppender (folder in core module) - Internal to opentelemetry-android. Provides a custom span processor for adding network attributes to all spans.
The above division of separate work in separate folders/modules makes sense and is also needed to have a clean dependency graph among them all. There are a few minor things I think we should refactor to make it more readable/usable:
- Rename instrumentation module to something better than what it currently is -
"network". It can be confused for network request interception. Can we rename it to something like -"networkchange"? - Can we drop
"internal/features"or just drop"features"if we still want to keep it internal from/common/main/java/io/opentelemetry/android/common/internal/features/networkattributes/? - Extract
NetworkAttributesSpanAppenderand all other span processors like ScreenAttributesSpanProcessor,SessionIdSpanAppenderandGlobalAtributesSpanAppenderin their own module to allow for reusability? (Contrary to the span processors, all log processors do have a house at a single place. They can perhaps be extracted in the same new module or a separate one for them?) - A small duplication I noticed - Instead of duplicating NETWORK_STATUS_KEY in NetworkApplicationListener,, NetworkChangeMonitorTest can utilize the same from NetworkChangeAttributesExtractor if feasible.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by comparing the instrumentation/network, services network, common/internal/features/networkattributes, and core/internal/features/networkattrs paths, along with NetworkApplicationListener, NetworkChangeMonitorTest, and NetworkChangeAttributesExtractor. Review the module dependency graph and determine which refactor, if any, has an agreed scope; done requires a specific migration plan and updated tests for the chosen changes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- android, kotlin
- Domain
- build-system, developer-experience, tooling
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 20/100