Skip to content

Conversation

@phacops
Copy link
Contributor

@phacops phacops commented Aug 14, 2025

It was decided logs wouldn't get a downsampled retention just yet, so we need to default to using the full fidelity retention now.

Spans keep the downsampled retention. No other TraceItem should have it.

@phacops phacops requested a review from a team as a code owner August 14, 2025 23:28
cursor[bot]

This comment was marked as outdated.

@phacops phacops changed the title fix(logs): Use full fidelity retention for logs until we decide otherwise fix(logs): Set logs retention to 30 days for full fidelity and downsampled datasets Aug 16, 2025
// Hard code both retentions for logs launch until we have separate retentions for
// different data categories.
#[cfg(feature = "processing")]
retention: _ctx.project_info.config.event_retention,
Copy link
Member

Choose a reason for hiding this comment

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

Now the function no longer needs _ctx as an argument.

#[cfg(feature = "processing")]
downsampled_retention: _ctx.project_info.config.downsampled_event_retention,
logs: all_logs,
downsampled_retention: Some(30),
Copy link
Member

Choose a reason for hiding this comment

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

We can just remove downsampled_retention and retention alltogether from the ExpandedLogs struct now and then have the retentions as a const somewhere in logs/store.rs.

@phacops phacops enabled auto-merge August 25, 2025 07:58
@phacops phacops disabled auto-merge August 25, 2025 07:58
@phacops phacops enabled auto-merge August 25, 2025 07:59
@phacops phacops added this pull request to the merge queue Aug 25, 2025
Merged via the queue into master with commit 97f49ca Aug 25, 2025
29 checks passed
@phacops phacops deleted the pierre/change-downsampled-retention-for-logs branch August 25, 2025 09:09
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.

3 participants