Skip to content

Conversation

@cade-conklin
Copy link

Links

Details

Issue

There is currently no way to locally configure the maximum for Transaction Events, Custom Insights Events, Error Events, and Log Events. Additionally, there are many functions that are essentially copied and renamed. There is also no handling of invalid config values at the time of setting (but rather later)

Goals

  • Allow user to configure the maximum for Transaction Events, Custom Insights Events, Error Events, and Log events through both environment variables and config options (functions).
  • Refactor reused code
  • Move logic for invalid config values to when those values are set (either by config options or environment variables)

Implementation

  • Added config option for ConfigTransactionEventsMaxSamplesStored
  • (Config Options)ConfigTransactionEventsMaxSamplesStored, ConfigCustomInsightsEventsMaxSamplesStored, ConfigErrorCollectorMaxSamplesStored, and ConfigAppLogForwardingMaxSamplesStored now use function maxConfigEvents(limit, max) to handle invalid configurations when being set
  • Added environment variablesNEW_RELIC_TRANSACTION_EVENTS_MAX_SAMPLES_STORED and NEW_RELIC_ERROR_COLLECTOR_MAX_EVENT_SAMPLES_STORED
  • (Env Vars) NEW_RELIC_APPLICATION_LOGGING_FORWARDING_MAX_SAMPLES_STORED, NEW_RELIC_TRANSACTION_EVENTS_MAX_SAMPLES_STORED, NEW_RELIC_ERROR_COLLECTOR_MAX_EVENT_SAMPLES_STORED, and NEW_RELIC_CUSTOM_INSIGHTS_EVENTS_MAX_SAMPLES_STORED now use assignIntWithMax(field, name, max) when being assigned to handle invalid configurations
  • Deleted maxCustomEvents, maxTxnEvents, and maxLogEvents as their logic was moved to maxConfigEvents and is called during assignment of config variables
  • Deleted MaxTxnEvents, MaxCustomEvents, MaxLogEvents, MaxSpanEvents, and MaxErrorEvents because they were all making a call to run.limit
  • Where the above ^ functions were called, they were now replaced with run.limit
  • run.limit directly passes in the config values instead of functions to get a valid value

How to test

More tests to come...
make core-suite

@cade-conklin cade-conklin changed the base branch from master to develop November 4, 2025 18:07
@codecov-commenter
Copy link

codecov-commenter commented Nov 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.06%. Comparing base (f534729) to head (d699ccd).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1103      +/-   ##
===========================================
+ Coverage    78.95%   79.06%   +0.11%     
===========================================
  Files          159      159              
  Lines        14919    14928       +9     
===========================================
+ Hits         11779    11803      +24     
+ Misses        2727     2713      -14     
+ Partials       413      412       -1     

see 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f534729...d699ccd. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment on lines -391 to -405
func TestConfigurableTxnEvents_configMoreThanMax(t *testing.T) {
h, err := internal.UnmarshalConnectReply([]byte(
`{"return_value":{
"event_harvest_config": {
"report_period_ms": 10000
}
}}`), internal.PreconnectReply{})
if nil != err {
t.Fatal(err)
}
cfg := config{Config: defaultConfig()}
cfg.TransactionEvents.MaxSamplesStored = internal.MaxTxnEvents + 100
result := newAppRun(cfg, h).MaxTxnEvents()
if result != internal.MaxTxnEvents {
t.Errorf("Unexpected max number of txn events, expected %d but got %d", internal.MaxTxnEvents, result)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why was this test removed?

// of how many custom events are stored in an agent for a given harvest cycle.
// Alters the CustomInsightsEvents.MaxSamplesStored setting.
// Note: As of Jul 2022, the absolute maximum events that can be sent each minute is 100000.
// Note: As of Oct 2025, the absolute maximum events that can be sent each minute is 30000.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The spec for custom insights states that the maximum number of events is 100000, not 30000

// of how many transaction events are stored in an agent for a given harvest cycle.
// Alters the TransactionEvents.MaxSamplesStored setting.
// Note: As of Oct 2025, the absolute maximum events that can be sent each minute is 10000.
func ConfigTransactionEventsMaxSamplesStored(limit int) ConfigOption {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: the variable limit in this context is a little confusing to me- at a glance, it implies that this function is receiving some kind of defined, canonical limit value rather than a user-desired config value for these settings. We may want to consider renaming this variable name to something like configured or just value while we're already changing the functions.

}
}

func TestConfigMaxTxnEvents(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have any other tests that validate the defaultConfig() function call behavior to set the cfg to the internal limits? I think we still tests to verify that the default config values behave as expected

Copy link
Collaborator

Choose a reason for hiding this comment

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

This one is specific to Txn events but all event types should have at least a single test to verify that the default value exists when nothing is explicitly set and the values match expectations.

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