Skip to content

Commit

Permalink
Fix #2248 duplicate key errors on dropped span stats update (#2283)
Browse files Browse the repository at this point in the history
* Fix #2248 duplicate key errors on dropped span stats update

* remove erroneous additional lock inside update lambda

* dotnet format

* Ensure we still respect max count of 128
  • Loading branch information
Mpdreamz authored Feb 19, 2024
1 parent be1655d commit b505dcd
Showing 1 changed file with 20 additions and 23 deletions.
43 changes: 20 additions & 23 deletions src/Elastic.Apm/Model/Transaction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ private void CheckAndCaptureBaggage()
/// <summary>
/// Internal dictionary to keep track of and look up dropped span stats.
/// </summary>
private Dictionary<DroppedSpanStatsKey, DroppedSpanStats> _droppedSpanStatsMap;
private ConcurrentDictionary<DroppedSpanStatsKey, DroppedSpanStats> _droppedSpanStatsMap;

private bool _isEnded;

Expand Down Expand Up @@ -552,38 +552,35 @@ private Activity StartActivity(bool shouldRestartTrace)
return activity;
}

private readonly object _lock = new();
internal void UpdateDroppedSpanStats(string serviceTargetType, string serviceTargetName, string destinationServiceResource, Outcome outcome,
double duration
)
{
//lock the lazy initialization of the dictionary
if (_droppedSpanStatsMap == null)
{
_droppedSpanStatsMap = new Dictionary<DroppedSpanStatsKey, DroppedSpanStats>
{
{
new DroppedSpanStatsKey(serviceTargetType, serviceTargetName, outcome),
new DroppedSpanStats(serviceTargetType, serviceTargetName, destinationServiceResource, outcome, duration)
}
};
lock (_lock)
_droppedSpanStatsMap ??= new ConcurrentDictionary<DroppedSpanStatsKey, DroppedSpanStats>();
}
else

lock (_lock)
{
if (_droppedSpanStatsMap.Count >= 128)
return;

if (_droppedSpanStatsMap.TryGetValue(new DroppedSpanStatsKey(serviceTargetType, serviceTargetName, outcome), out var item))
{
item.Duration ??=
new DroppedSpanStats.DroppedSpanDuration { Sum = new DroppedSpanStats.DroppedSpanDuration.DroppedSpanDurationSum() };

item.Duration.Count++;
item.Duration.Sum.UsRaw += duration;
}
else
{
_droppedSpanStatsMap.Add(new DroppedSpanStatsKey(serviceTargetType, serviceTargetName, outcome),
new DroppedSpanStats(serviceTargetType, serviceTargetName, destinationServiceResource, outcome, duration));
}
//AddOrUpdate callbacks can run multiple times so still wrapping this in a lock
var key = new DroppedSpanStatsKey(serviceTargetType, serviceTargetName, outcome);
_droppedSpanStatsMap.AddOrUpdate(key,
_ => new DroppedSpanStats(serviceTargetType, serviceTargetName, destinationServiceResource, outcome, duration),
(_, stats) =>
{
stats.Duration ??=
new DroppedSpanStats.DroppedSpanDuration { Sum = new DroppedSpanStats.DroppedSpanDuration.DroppedSpanDurationSum() };
stats.Duration.Count++;
stats.Duration.Sum.UsRaw += duration;
return stats;
});
}
}

Expand Down

0 comments on commit b505dcd

Please sign in to comment.