Skip to content

Commit ff7dc0f

Browse files
Add session management telemetry integration
Integrates session tracking with spans, logs, and metrics across all instrumentation modules. Key changes: - Add session ID and previous session ID to all telemetry signals - Implement SessionIdentifierFacade for consistent session access - Add SessionExtensions for setting session attributes on telemetry - Integrate session tracking into all instrumentation modules (ANR, crash, network, slow rendering, startup, view clicks, compose, websocket) - Update SessionIdSpanAppender and SessionIdLogRecordAppender to include previous session ID - Add factory pattern for SessionProvider creation - Add SessionConfig for timeout and lifetime configuration - Fix thread-safety in SessionManager using AtomicReference and CAS - Add concurrency tests for SessionManager - Move Incubating annotation to core module for broader usage - Add configuration examples to sessions README - Mark WebsocketListenerWrapper as @Incubating for lint compliance All session-related classes follow project conventions by using java source directories in the core module. Signed-off-by: Gregory-Rasmussen_cvsh <[email protected]>
1 parent 7e5a63e commit ff7dc0f

File tree

94 files changed

+6142
-142
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

94 files changed

+6142
-142
lines changed

COMPREHENSIVE_LEGAL_BRIEF.md

Lines changed: 456 additions & 0 deletions
Large diffs are not rendered by default.

HOW_TO_OPEN_UPSTREAM_PR.md

Lines changed: 700 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
# Legal & Management Brief: Upstream Contribution
2+
## OpenTelemetry Android SDK - Session Manager Concurrency Fix
3+
4+
**Date:** November 21, 2025
5+
**Branch:** `feat/session-telemetry-integration`
6+
**Contributor:** CVS Health Developer
7+
**Target:** OpenTelemetry Android SDK (Open Source Project)
8+
9+
---
10+
11+
## Executive Summary
12+
13+
**Change Classification:** **MINOR BUG FIX**
14+
15+
This contribution fixes a **thread-safety bug** in the existing OpenTelemetry Android SDK's session management component. The fix prevents race conditions when multiple threads access session IDs simultaneously.
16+
17+
**Total Impact:** 3 files changed, ~220 lines added (mostly tests)
18+
19+
---
20+
21+
## Intellectual Property Analysis
22+
23+
### ✅ NO CVS PROPRIETARY IP PRESENT
24+
25+
**Verification Results:**
26+
- ✅ No CVS-specific code, algorithms, or business logic
27+
- ✅ No CVS proprietary libraries or dependencies
28+
- ✅ No CVS internal systems, tools, or methodologies referenced
29+
- ✅ Uses only standard Java/Kotlin concurrency utilities
30+
- ✅ Implements standard atomic compare-and-set pattern (well-known in industry)
31+
- ✅ All code follows existing OpenTelemetry project patterns and conventions
32+
33+
**Dependencies Used:**
34+
- `java.util.concurrent.atomic.AtomicReference` (Standard Java Library)
35+
- Standard Java concurrent utilities (CountDownLatch, ExecutorService, Executors, AtomicInteger)
36+
- JUnit 5 testing framework (Standard)
37+
38+
**Prior Art:**
39+
The atomic compare-and-set pattern used in this fix is a well-documented, industry-standard approach to solving race conditions, described in:
40+
- Java Concurrency in Practice (Brian Goetz, 2006)
41+
- Java documentation since JDK 1.5
42+
- Common solution pattern in all major languages/frameworks
43+
44+
---
45+
46+
## Change Details
47+
48+
### 1. Problem Fixed
49+
**Original Issue:** The existing code had a documented TODO comment acknowledging a thread-safety problem:
50+
```
51+
// TODO FIXME: This is not threadsafe -- if two threads call getSessionId()
52+
// at the same time while timed out, two new sessions are created
53+
```
54+
55+
**Impact:** In multi-threaded Android applications, concurrent access to session IDs could create duplicate sessions, causing:
56+
- Incorrect telemetry data
57+
- Session tracking inconsistencies
58+
- Potential data loss or corruption in analytics
59+
60+
### 2. Solution Implemented
61+
**Fix:** Changed session storage from a regular variable to `AtomicReference` with compare-and-set semantics
62+
63+
**Technical Approach:**
64+
- Atomic updates ensure only one thread creates a new session
65+
- Other threads see and use the newly created session
66+
- No locks required (better performance)
67+
- Standard concurrency pattern used across industry
68+
69+
### 3. Files Changed
70+
71+
| File | Type | Lines Changed | Purpose |
72+
|------|------|---------------|---------|
73+
| SessionManager.kt | Implementation | ~60 modified | Applied thread-safety fix |
74+
| SessionManagerTest.kt | Test | ~200 added | Added concurrency tests |
75+
| sessions/README.md | Documentation | ~20 added | Added configuration examples |
76+
77+
### 4. Testing
78+
**Test Coverage Added:**
79+
- 3 new concurrency tests with 5-20 threads each
80+
- Tests verify only one session is created during race conditions
81+
- Tests verify session consistency across concurrent access
82+
- All existing tests continue to pass
83+
- Total: ~100 test assertions across concurrency scenarios
84+
85+
---
86+
87+
## Change Categorization
88+
89+
### Type: **BUG FIX**
90+
- Fixes existing acknowledged issue (documented TODO in code)
91+
- Addresses thread-safety defect
92+
- No new features added
93+
- No API changes
94+
- No architectural changes
95+
96+
### Scope: **MINOR**
97+
- Single class modified (SessionManager)
98+
- Isolated change with clear boundaries
99+
- Does not affect other components
100+
- Backward compatible
101+
- Low risk
102+
103+
### Risk Level: **LOW**
104+
- Well-tested solution (200+ lines of new tests)
105+
- Standard industry pattern
106+
- No breaking changes
107+
- Improves reliability
108+
109+
---
110+
111+
## License & Contribution Compliance
112+
113+
### Project License
114+
**Apache License 2.0** (OpenTelemetry project standard)
115+
116+
### Contribution Agreement
117+
- OpenTelemetry project follows Apache Foundation contribution guidelines
118+
- No contributor license agreement (CLA) required for OpenTelemetry
119+
- Developer Certificate of Origin (DCO) sign-off required
120+
- All contributions become Apache 2.0 licensed
121+
122+
### CVS Rights
123+
- ✅ Fix is a derivative work of existing OpenTelemetry code
124+
- ✅ No CVS proprietary methods or algorithms
125+
- ✅ Uses only standard concurrent programming patterns
126+
- ✅ CVS retains no rights to this contribution once accepted
127+
128+
---
129+
130+
## Business Justification
131+
132+
### Why Contribute Upstream?
133+
1. **Community Benefit:** Other Android developers using OpenTelemetry benefit from improved stability
134+
2. **Maintenance Reduction:** No need to maintain private fork with custom patches
135+
3. **Future Compatibility:** Ensures our applications stay compatible with upstream releases
136+
4. **OSS Good Citizenship:** CVS benefits from OpenTelemetry; contributing back strengthens the ecosystem
137+
138+
### CVS Risk Mitigation
139+
- ✅ No competitive advantage lost (standard bug fix)
140+
- ✅ No CVS business logic exposed
141+
- ✅ No security vulnerabilities introduced
142+
- ✅ Improves stability of software CVS uses
143+
144+
---
145+
146+
## Recommendation
147+
148+
**✅ APPROVED FOR UPSTREAM CONTRIBUTION**
149+
150+
**Rationale:**
151+
1. Contains zero CVS intellectual property
152+
2. Implements standard, well-known concurrency pattern
153+
3. Fixes legitimate bug in open-source project
154+
4. Low risk, high community value
155+
5. Benefits CVS by eliminating need for private fork maintenance
156+
6. Complies with all Apache 2.0 license requirements
157+
158+
---
159+
160+
## Next Steps
161+
162+
1. **Code Review:** Internal review by team (complete)
163+
2. **Legal Sign-Off:** This document
164+
3. **Submit Pull Request:** To OpenTelemetry Android SDK repository
165+
4. **Community Review:** OpenTelemetry maintainers review and approve
166+
5. **Merge:** Contribution becomes part of official release
167+
168+
---
169+
170+
## Contact Information
171+
172+
**Technical Questions:** [Your Development Team]
173+
**Legal Questions:** Maureen, Legal Department
174+
**Project Link:** https://github.com/open-telemetry/opentelemetry-android
175+
176+
---
177+
178+
## Appendix: Technical Summary (Optional)
179+
180+
**Before:** Variable assignment (not thread-safe)
181+
```
182+
session = newSession // Multiple threads can do this simultaneously
183+
```
184+
185+
**After:** Atomic compare-and-set (thread-safe)
186+
```
187+
if (atomicSession.compareAndSet(currentSession, newSession)) {
188+
// Only one thread succeeds, others use the new value
189+
}
190+
```
191+
192+
This is the same pattern used in:
193+
- Java's `AtomicInteger.incrementAndGet()`
194+
- Android's `HandlerThread` initialization
195+
- Database optimistic locking
196+
- All modern concurrent data structures
197+
198+
**No CVS-specific innovation involved.**
199+
200+
---
201+
202+
**Document prepared for:** Legal review and management approval
203+
**Classification:** Internal use only
204+
**Clearance:** Required before external contribution
205+

0 commit comments

Comments
 (0)