Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase custom_insights_events.max_samples_stored #1541

Merged
merged 1 commit into from
Oct 13, 2022

Conversation

kaylareopelle
Copy link
Contributor

@kaylareopelle kaylareopelle commented Oct 13, 2022

Overview

The previous default value for custom_insights_events.max_samples_stored was 1000. The new default is 3000.

This new number is based on OATS results from multiple language agents in an effort to reduce the number of dropped custom insights events.

Closes #1540

@kaylareopelle kaylareopelle force-pushed the custom_events_reservoir_limit branch from b2de8bc to cb34d08 Compare October 13, 2022 20:23
@kaylareopelle kaylareopelle marked this pull request as ready for review October 13, 2022 20:24
tannalynn
tannalynn previously approved these changes Oct 13, 2022
Copy link
Contributor

@tannalynn tannalynn left a comment

Choose a reason for hiding this comment

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

🎉

fallwith
fallwith previously approved these changes Oct 13, 2022
The previous default value was 1000. The new default is 3000.

This new number is based on  OATS results from multiple language agents
@kaylareopelle kaylareopelle dismissed stale reviews from fallwith and tannalynn via cbc5b38 October 13, 2022 20:38
@kaylareopelle kaylareopelle force-pushed the custom_events_reservoir_limit branch from cb34d08 to cbc5b38 Compare October 13, 2022 20:38
@github-actions
Copy link
Contributor

SimpleCov Report

Coverage Threshold
Line 93.26% 93%
Branch 84.14% 84%

@kaylareopelle kaylareopelle merged commit 21ab8be into dev Oct 13, 2022
@kaylareopelle kaylareopelle deleted the custom_events_reservoir_limit branch October 13, 2022 20:51
@@ -2004,7 +2004,7 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil)
:description => 'If `true`, the agent captures [custom events](/docs/insights/new-relic-insights/adding-querying-data/inserting-custom-events-new-relic-apm-agents).'
},
:'custom_insights_events.max_samples_stored' => {
:default => 1000,
:default => 3000,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any impact to increasing this number? I'm wondering how we can up with 1000 in the first place, and why 3000 is the new number.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Hannah! Please see this slack thread (internal)

1000 is actually lower than what all the other agents have at this time. The spec default is 10K per limit, so we're getting closer to the spec with a smaller performance impact by changing the value to 3K.

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.

Increase Custom Event Reservoir Default Max Samples Stored to 3,000
4 participants