Skip to content

Commit

Permalink
Merge pull request #1784 from newrelic/1757_LogSender-sends-too-much
Browse files Browse the repository at this point in the history
1757 log sender sends too much
  • Loading branch information
jbedell-newrelic authored Mar 6, 2024
2 parents cb8f2a8 + b9d8a13 commit bfcd92a
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public void configure(long reportPeriodInMillis, int maxSamplesStored) {

if (maxSamplesStored != service.getMaxSamplesStored()) {
service.setMaxSamplesStored(maxSamplesStored);
service.setReportPeriodInMillis(reportPeriodInMillis);
maybeSendSpanLimitMetric(maxSamplesStored);
service.harvestEvents(appName);
service.clearReservoir();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@ public interface EventService extends Service {
*/
void setMaxSamplesStored(int maxSamplesStored);

/**
* Update the reporting period for harvesting, in case it is needed for config changes.
*
* @param reportPeriodInMillis the number of millis between reporting/harvesting cycles
*/
default void setReportPeriodInMillis(long reportPeriodInMillis) {
// do nothing
}

/**
* Reset the event reservoir to allow for the next harvest to start
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ public class LogSenderServiceImpl extends AbstractService implements LogSenderSe
private final ConcurrentMap<String, Boolean> isEnabledForApp = new ConcurrentHashMap<>();
// Number of log events in the reservoir sampling buffer per-app. All apps get the same value.
private volatile int maxSamplesStored;
// Number of millis between harvest/reporting cycles.
// when the maxSamplesStores changes from the config file, it is set as per minute. This value is needed
// to properly calculate the per harvest cycle maxSamplesStored
// we'll default to 5000, unless overridden
volatile long reportPeriodInMillis = 5000;
// Key is app name, value is collection of per-transaction log events for next harvest for that app.
private final ConcurrentHashMap<String, DistributedSamplingPriorityQueue<LogEvent>> reservoirForApp = new ConcurrentHashMap<>();

Expand Down Expand Up @@ -110,7 +115,7 @@ public void configChanged(String appName, AgentConfig agentConfig) {
// if the config has changed for the app, just remove it and regenerate enabled next transaction
isEnabledForApp.remove(appName);

maxSamplesStored = appLoggingConfig.getMaxSamplesStored();
maxSamplesStored = (int) (appLoggingConfig.getMaxSamplesStored()*(reportPeriodInMillis / 60000.0));
forwardingEnabled = appLoggingConfig.isForwardingEnabled();
contextDataKeyFilter = createContextDataKeyFilter(appLoggingConfig);

Expand Down Expand Up @@ -149,7 +154,7 @@ public LogSenderServiceImpl() {
AgentConfig config = ServiceFactory.getConfigService().getDefaultAgentConfig();
ApplicationLoggingConfig appLoggingConfig = config.getApplicationLoggingConfig();

maxSamplesStored = appLoggingConfig.getMaxSamplesStored();
maxSamplesStored = (int) (appLoggingConfig.getMaxSamplesStored()*(reportPeriodInMillis / 60000.0));
forwardingEnabled = appLoggingConfig.isForwardingEnabled();
contextDataKeyFilter = createContextDataKeyFilter(appLoggingConfig);

Expand Down Expand Up @@ -280,6 +285,10 @@ public void setMaxSamplesStored(int maxSamplesStored) {
this.maxSamplesStored = maxSamplesStored;
}

public void setReportPeriodInMillis(long reportPeriodInMillis) {
this.reportPeriodInMillis = reportPeriodInMillis;
}

public void clearReservoir() {
reservoirForApp.clear();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,13 @@ private static LogSenderServiceImpl createService(Map<String, Object> config) th
public void testHarvestableConfigure() throws Exception {
Map<String, Object> config = createConfig(true, 180);
LogSenderServiceImpl logSenderService = createService(config);
assertEquals(833, logSenderService.getMaxSamplesStored());
assertEquals(5000, logSenderService.reportPeriodInMillis);

logSenderService.addHarvestableToService(appName);
logSenderService.configureHarvestables(60, 1);

assertEquals(1, logSenderService.getMaxSamplesStored());
assertEquals(60, logSenderService.reportPeriodInMillis);
}

@Test
Expand Down

0 comments on commit bfcd92a

Please sign in to comment.