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.

Sign up for GitHub

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

Make sure BreakdownMetricsProvider prints 1K warning only once per collection #1367

Merged
merged 1 commit into from
Aug 4, 2021
Merged

Make sure BreakdownMetricsProvider prints 1K warning only once per collection #1367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions src/Elastic.Apm/Metrics/MetricsProvider/BreakdownMetricsProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
// See the LICENSE file in the project root for more information

using System.Collections.Generic;
using System.Linq;
using Elastic.Apm.Api;
using Elastic.Apm.Helpers;
using Elastic.Apm.Logging;
Expand All @@ -19,6 +18,11 @@ internal class BreakdownMetricsProvider : IMetricsProvider
private readonly List<MetricSet> _itemsToSend = new();
private readonly IApmLogger _logger;

/// <summary>
/// Indicates if the 10K limit log was already printed.
/// </summary>
private bool loggedWarning = false;

private readonly object _lock = new();
private int _transactionCount;
public int ConsecutiveNumberOfFailedReads { get; set; }
Expand All @@ -44,7 +48,6 @@ public void CaptureTransaction(Transaction transaction)
{
_transactionCount++;
var timestampNow = TimeUtils.TimestampNow();
var loggedWarning = false;

foreach (var item in transaction.SpanTimings)
{
Expand Down Expand Up @@ -89,7 +92,7 @@ public void CaptureTransaction(Transaction transaction)
{
_logger.Warning()
?.Log(
"The limit of 1000 metricsets has been reached, no new metricsets will be created.");
"The limit of 1000 metricsets has been reached, no new metricsets will be created until the current set is sent to APM Server.");
}
}
}
Expand All @@ -104,6 +107,7 @@ public IEnumerable<MetricSet> GetSamples()
retVal.AddRange(_itemsToSend);
_itemsToSend.Clear();
_transactionCount = 0;
loggedWarning = false;
}

return retVal;
Expand Down
50 changes: 45 additions & 5 deletions test/Elastic.Apm.Tests/BreakdownTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@
using System;
using System.Linq;
using System.Threading;
using Elastic.Apm.Config;
using Elastic.Apm.Logging;
using Elastic.Apm.Metrics;
using Elastic.Apm.Metrics.MetricsProvider;
using Elastic.Apm.Model;
using Elastic.Apm.ServerInfo;
using Elastic.Apm.Tests.Utilities;
using FluentAssertions;
Expand Down Expand Up @@ -673,14 +672,55 @@ public void MoreThan1KBreakDownMetrics()
breakdownMetricsProvider.GetSamples().Count().Should().Be(1000);
}

/// <summary>
/// Makes sure the 1K limit warning in <see cref="BreakdownMetricsProvider" /> is only printed once per metric collection
/// (and e.g. not per transaction).
/// See https://github.com/elastic/apm-agent-dotnet/issues/1361.
/// </summary>
[Fact]
public void BreakdownLogTest()
{
var testLogger = new TestLogger(LogLevel.Warning);
var rnd = new Random();

var (agent, breakdownMetricsProvider) = SetUpAgent(testLogger);
using (agent)
{
for (var transactionNumber = 0; transactionNumber < 50; transactionNumber++)
{
var t = agent.TracerInternal.StartTransactionInternal("test", "request");


for (var i = 0; i < 5000; i++) t.CaptureSpan("foo", $"bar-{rnd.Next().ToString()}", () => { });
t.End();
}
}

// Make sure the 1K limit warning is only logged once
testLogger.Lines.Count(n => n.Contains("The limit of 1000 metricsets has been reached, no new metricsets will be created"))
.Should()
.Be(1);
breakdownMetricsProvider.GetSamples().Count().Should().Be(1000);

var t2 = agent.TracerInternal.StartTransactionInternal("test", "request");
for (var i = 0; i < 5000; i++) t2.CaptureSpan("foo", $"bar-{rnd.Next().ToString()}", () => { });
t2.End();

// After BreakdownMetricsProvider.GetSamples() the warning is logged again
testLogger.Lines.Count(n => n.Contains("The limit of 1000 metricsets has been reached, no new metricsets will be created"))
.Should()
.Be(2);
}

private bool DoubleCompare(double value, double expectedValue) => Math.Abs(value - expectedValue) < 1000;

private (ApmAgent, BreakdownMetricsProvider) SetUpAgent()
private (ApmAgent, BreakdownMetricsProvider) SetUpAgent(IApmLogger logger = null)
{
var breakdownMetricsProvider = new BreakdownMetricsProvider(new NoopLogger());
logger ??= new NoopLogger();
var breakdownMetricsProvider = new BreakdownMetricsProvider(logger);

var agentComponents = new AgentComponents(
new NoopLogger(),
logger,
new MockConfigSnapshot(metricsInterval: "1s"),
new NoopPayloadSender(),
new FakeMetricsCollector(), //metricsCollector will be set in AgentComponents.ctor
Expand Down