Skip to content

NR-502468: Fix session attributes persistence and time conversion #604

Open
diegomtz5 wants to merge 4 commits intodevelopfrom
NR-502468
Open

NR-502468: Fix session attributes persistence and time conversion #604
diegomtz5 wants to merge 4 commits intodevelopfrom
NR-502468

Conversation

@diegomtz5
Copy link

Summary

Fixes session attributes not being sent consistently across harvests, ensuring platformVersion is used for
instrumentation.version instead of falling back to native agent version. Session attributes were only populated when handleHarvest() was triggered by specific conditions. Harvests containing only metrics/HTTP requests (no events) sent empty session attributes {}.

Changes

1. Preserve Event Timestamp Across Harvests

  • Ensures session attributes sent on every harvest after first event harvest
  • Remove timestamp reset from empty() method (both event systems)
  • Add resetTimestamp() method for explicit session clear
  • Matches Android behavior

2. Fix Logger Instrumentation Version

  • Update NRLogger.m to use platformVersion with fallback to agentVersion
  • Ensures logs report correct hybrid agent version

3. Fix Time Conversion Constant

  • Replace kDefaultBufferSize (event count) with kMillisecondsPerSecond in didReachMaxQueueTime()
  • Semantically correct - was accidentally using event count constant for time conversion
  • No behavior change (both equal 1000), but now clear and correct

Testing

  • Updated and added tests for both event systems
  • Verifies timestamp persistence and explicit reset behavior

diegomtz5 and others added 2 commits February 18, 2026 09:28
- Remove timestamp reset from empty() method in both event systems
- Add resetTimestamp() method for explicit session clear
- Update tests for new behavior
- Ensures session attributes sent on every harvest after first 60s

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Replace kDefaultBufferSize (event count) with kMillisecondsPerSecond
- kDefaultBufferSize was semantically incorrect for time conversion
- Both equal 1000 so behavior unchanged, but now semantically correct

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

ASSERT_THROW(manager.newEvent(strs2), std::runtime_error);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I forgot about libMobileAgent C++ tests, they will go away once we make NewEventSystem default and remove the new event system flag. Excellent work getting coverage.

Copy link
Member

@cdillard-NewRelic cdillard-NewRelic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good I ran your branch with NewEventSystem enabled and now see session attributes on every event harvest. I ran you rbranch with the old event system and everything appears to work the same as before. This seems ready to merge when you are!

@Newrelic-Boomi
Copy link

Updates session replay to report correct platform information for hybrid agents:
- instrumentation.name: Uses platform-aware logic (native vs hybrid)
- instrumentation.version: Uses platformVersion with fallback to agentVersion
- Matches NRLogger pattern for consistent reporting across all data types

This ensures hybrid agents (React Native, Flutter, Capacitor, etc.) are
correctly identified with their platform version in session replay data.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@diegomtz5
Copy link
Author

Added the changes to the SessionReplayReporter for instrumentation.name, instrumentation.version and collector.name, to match everything else.

@cdillard-NewRelic
Copy link
Member

Awesome use of blocks in SessionReplay 👍

Copy link
Member

@cdillard-NewRelic cdillard-NewRelic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments