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

chore: Fix MetricName conflict in MetricWireModel. #2112

Merged
merged 1 commit into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,7 @@ private void TrySend(MetricWireModel metric)

if (_publishMetricDelegate == null)
{
Log.Warn("No PublishMetricDelegate to flush metric '{0}' through.", metric.MetricName.Name);
Log.Warn("No PublishMetricDelegate to flush metric '{0}' through.", metric.MetricNameModel.Name);
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ public static void WriteJsonImpl(JsonWriter jsonWriter, MetricWireModel value, J
{
jsonWriter.WriteStartArray();

MetricNameWireModelJsonConverter.WriteJsonImpl(jsonWriter, value.MetricName, serializer);
MetricNameWireModelJsonConverter.WriteJsonImpl(jsonWriter, value.MetricNameModel, serializer);

MetricDataWireModelJsonConverter.WriteJsonImpl(jsonWriter, value.Data, serializer);
MetricDataWireModelJsonConverter.WriteJsonImpl(jsonWriter, value.DataModel, serializer);

jsonWriter.WriteEndArray();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ private void TrySend(MetricWireModel metric)

if (_publishMetricDelegate == null)
{
Log.Warn("No PublishMetricDelegate to flush metric '{0}' through.", metric.MetricName.Name);
Log.Warn("No PublishMetricDelegate to flush metric '{0}' through.", metric.MetricNameModel.Name);
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ private void TrySend(MetricWireModel metric)

if (_publishMetricDelegate == null)
{
Log.Warn("No PublishMetricDelegate to flush metric '{0}' through.", metric.MetricName.Name);
Log.Warn("No PublishMetricDelegate to flush metric '{0}' through.", metric.MetricNameModel.Name);
return;
}

Expand Down
39 changes: 19 additions & 20 deletions src/Agent/NewRelic/Agent/Core/WireModels/MetricWireModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,19 @@
using System.Collections.Generic;
using System.Linq;
using System.Net;
using InternalMetricName = NewRelic.Agent.Core.Metrics.MetricName;

namespace NewRelic.Agent.Core.WireModels
{
[JsonConverter(typeof(MetricWireModelJsonConverter))]
public class MetricWireModel : IAllMetricStatsCollection
{
public readonly MetricNameWireModel MetricName;
public readonly MetricDataWireModel Data;
public readonly MetricNameWireModel MetricNameModel;
public readonly MetricDataWireModel DataModel;

private MetricWireModel(MetricNameWireModel metricName, MetricDataWireModel data)
private MetricWireModel(MetricNameWireModel metricNameModel, MetricDataWireModel dataModel)
{
MetricName = metricName;
Data = data;
MetricNameModel = metricNameModel;
DataModel = dataModel;
}

/// <summary>
Expand All @@ -55,13 +54,13 @@ public static MetricWireModel Merge(IEnumerable<MetricWireModel> metrics)
throw new Exception("At least one metric must be passed in");
}

var metricName = metrics.First().MetricName;
if (metrics.Any(metric => !metric.MetricName.Equals(metricName)))
var metricName = metrics.First().MetricNameModel;
if (metrics.Any(metric => !metric.MetricNameModel.Equals(metricName)))
{
throw new Exception("Cannot merge metrics with different names");
}

var inputData = metrics.Select(metric => metric.Data);
var inputData = metrics.Select(metric => metric.DataModel);
var mergedData = MetricDataWireModel.BuildAggregateData(inputData);
return new MetricWireModel(metricName, mergedData);
}
Expand All @@ -81,18 +80,18 @@ public static MetricWireModel BuildMetric(IMetricNameService metricNameService,

public override string ToString()
{
return MetricName + Data.ToString();
return MetricNameModel + DataModel.ToString();
}

public void AddMetricsToCollection(MetricStatsCollection collection)
{
if (string.IsNullOrEmpty(MetricName.Scope))
if (string.IsNullOrEmpty(MetricNameModel.Scope))
{
collection.MergeUnscopedStats(MetricName.Name, Data);
collection.MergeUnscopedStats(MetricNameModel.Name, DataModel);
}
else
{
collection.MergeScopedStats(MetricName.Scope, MetricName.Name, Data);
collection.MergeScopedStats(MetricNameModel.Scope, MetricNameModel.Name, DataModel);
}
}

Expand All @@ -105,7 +104,7 @@ public override bool Equals(object obj)

if (obj is MetricWireModel other)
{
return other.MetricName.Equals(this.MetricName) && other.Data.Equals(this.Data);
return other.MetricNameModel.Equals(this.MetricNameModel) && other.DataModel.Equals(this.DataModel);
}

return false;
Expand All @@ -114,8 +113,8 @@ public override bool Equals(object obj)
public override int GetHashCode()
{
var hashCode = 2074576463;
hashCode = hashCode * -1521134295 + EqualityComparer<MetricNameWireModel>.Default.GetHashCode(MetricName);
hashCode = hashCode * -1521134295 + EqualityComparer<MetricDataWireModel>.Default.GetHashCode(Data);
hashCode = hashCode * -1521134295 + EqualityComparer<MetricNameWireModel>.Default.GetHashCode(MetricNameModel);
hashCode = hashCode * -1521134295 + EqualityComparer<MetricDataWireModel>.Default.GetHashCode(DataModel);
return hashCode;
}

Expand All @@ -138,7 +137,7 @@ public static void TryBuildTransactionMetrics(bool isWebTransaction, TimeSpan re
? MetricNames.WebTransactionAll
: MetricNames.OtherTransactionAll;
txStats.MergeUnscopedStats(proposedName, data);
txStats.MergeUnscopedStats(InternalMetricName.Create(txStats.GetTransactionName().PrefixedName), data);
txStats.MergeUnscopedStats(MetricName.Create(txStats.GetTransactionName().PrefixedName), data);

// "HttpDispacher" is a metric that is used to populate the APM response time chart.
if (isWebTransaction)
Expand Down Expand Up @@ -247,7 +246,7 @@ public static void TryBuildApdexMetrics(string transactionApdexName, bool isWebT
TimeSpan apdexT, TransactionMetricStatsCollection txStats)
{
var data = MetricDataWireModel.BuildApdexData(responseTime, apdexT);
txStats.MergeUnscopedStats(InternalMetricName.Create(transactionApdexName), data);
txStats.MergeUnscopedStats(MetricName.Create(transactionApdexName), data);
txStats.MergeUnscopedStats(MetricNames.ApdexAll, data);
var proposedName = isWebTransaction
? MetricNames.ApdexAllWeb
Expand All @@ -264,7 +263,7 @@ public static void TryBuildFrustratedApdexMetrics(bool isWebTransaction, string
? MetricNames.ApdexAllWeb
: MetricNames.ApdexAllOther;
txStats.MergeUnscopedStats(proposedName, data);
txStats.MergeUnscopedStats(InternalMetricName.Create(txApdexName), data);
txStats.MergeUnscopedStats(MetricName.Create(txApdexName), data);
}

#endregion Transaction apdex builders
Expand Down Expand Up @@ -444,7 +443,7 @@ public static void TryBuildDatastoreRollupMetrics(DatastoreVendor vendor, TimeSp
public static void TryBuildDatastoreStatementMetric(DatastoreVendor vendor, ParsedSqlStatement sqlStatement,
TimeSpan totalTime, TimeSpan exclusiveDuration, TransactionMetricStatsCollection txStats)
{
var proposedName = InternalMetricName.Create(sqlStatement.DatastoreStatementMetricName);
var proposedName = MetricName.Create(sqlStatement.DatastoreStatementMetricName);
var data = MetricDataWireModel.BuildTimingData(totalTime, exclusiveDuration);
txStats.MergeUnscopedStats(proposedName, data);
txStats.MergeScopedStats(proposedName, data);
Expand Down
24 changes: 12 additions & 12 deletions tests/Agent/UnitTests/CompositeTests/Assertions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,18 @@ public static void MetricsExist(IEnumerable<ExpectedMetric> expectedMetrics, IEn
continue;
}

if ((expectedMetric.Value0 != null && matchedMetric.Data.Value0 != expectedMetric.Value0) ||
(expectedMetric.Value1 != null && matchedMetric.Data.Value1 != expectedMetric.Value1) ||
(expectedMetric.Value2 != null && matchedMetric.Data.Value2 != expectedMetric.Value2) ||
(expectedMetric.Value3 != null && matchedMetric.Data.Value3 != expectedMetric.Value3) ||
(expectedMetric.Value4 != null && matchedMetric.Data.Value4 != expectedMetric.Value4) ||
(expectedMetric.Value5 != null && matchedMetric.Data.Value5 != expectedMetric.Value5))
if ((expectedMetric.Value0 != null && matchedMetric.DataModel.Value0 != expectedMetric.Value0) ||
(expectedMetric.Value1 != null && matchedMetric.DataModel.Value1 != expectedMetric.Value1) ||
(expectedMetric.Value2 != null && matchedMetric.DataModel.Value2 != expectedMetric.Value2) ||
(expectedMetric.Value3 != null && matchedMetric.DataModel.Value3 != expectedMetric.Value3) ||
(expectedMetric.Value4 != null && matchedMetric.DataModel.Value4 != expectedMetric.Value4) ||
(expectedMetric.Value5 != null && matchedMetric.DataModel.Value5 != expectedMetric.Value5))
{
builder.AppendFormat("Metric named {0} scoped to {1} was found in the metric payload, but had unexpected stats.", matchedMetric.MetricName.Name, matchedMetric.MetricName.Scope ?? "nothing");
builder.AppendFormat("Metric named {0} scoped to {1} was found in the metric payload, but had unexpected stats.", matchedMetric.MetricNameModel.Name, matchedMetric.MetricNameModel.Scope ?? "nothing");
builder.AppendLine();
builder.AppendFormat("Expected: {0}, {1}, {2}, {3}, {4}, {5}", expectedMetric.Value0, expectedMetric.Value1, expectedMetric.Value2, expectedMetric.Value3, expectedMetric.Value4, expectedMetric.Value5);
builder.AppendLine();
builder.AppendFormat("Actual: {0}, {1}, {2}, {3}, {4}, {5}", matchedMetric.Data.Value0, matchedMetric.Data.Value1, matchedMetric.Data.Value2, matchedMetric.Data.Value3, matchedMetric.Data.Value4, matchedMetric.Data.Value5);
builder.AppendFormat("Actual: {0}, {1}, {2}, {3}, {4}, {5}", matchedMetric.DataModel.Value0, matchedMetric.DataModel.Value1, matchedMetric.DataModel.Value2, matchedMetric.DataModel.Value3, matchedMetric.DataModel.Value4, matchedMetric.DataModel.Value5);
builder.AppendLine();
succeeded = false;
}
Expand All @@ -62,7 +62,7 @@ public static void MetricsDoNotExist(IEnumerable<ExpectedMetric> unexpectedMetri

if (matchedMetric != null)
{
builder.AppendFormat("Metric named {0} scoped to {1} was found in the metric payload.", matchedMetric.MetricName.Name, matchedMetric.MetricName.Scope ?? "nothing");
builder.AppendFormat("Metric named {0} scoped to {1} was found in the metric payload.", matchedMetric.MetricNameModel.Name, matchedMetric.MetricNameModel.Scope ?? "nothing");
builder.AppendLine();
succeeded = false;
}
Expand All @@ -75,11 +75,11 @@ private static MetricWireModel TryFindMetric(ExpectedMetric expectedMetric, IEnu
{
foreach (var actualMetric in actualMetrics)
{
if (expectedMetric.IsRegexName && !Regex.IsMatch(actualMetric.MetricName.Name, expectedMetric.Name))
if (expectedMetric.IsRegexName && !Regex.IsMatch(actualMetric.MetricNameModel.Name, expectedMetric.Name))
continue;
if (!expectedMetric.IsRegexName && expectedMetric.Name != actualMetric.MetricName.Name)
if (!expectedMetric.IsRegexName && expectedMetric.Name != actualMetric.MetricNameModel.Name)
continue;
if (expectedMetric.Scope != actualMetric.MetricName.Scope)
if (expectedMetric.Scope != actualMetric.MetricNameModel.Scope)
continue;

return actualMetric;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ public void SimpleTransaction_EndedMultipleTimes()
Assert.AreEqual(1, transactionEvents.Count);
Assert.AreEqual("foregroundExternal", transactionEvents.First().AgentAttributes()["request.uri"]);
CollectionAssert.IsEmpty(errors);
CollectionAssert.IsEmpty(metrics.Where(x => x.MetricName.Name.Contains("backgroundExternal")));
CollectionAssert.IsNotEmpty(metrics.Where(x => x.MetricName.Name.Contains("foregroundExternal")));
CollectionAssert.IsEmpty(metrics.Where(x => x.MetricNameModel.Name.Contains("backgroundExternal")));
CollectionAssert.IsNotEmpty(metrics.Where(x => x.MetricNameModel.Name.Contains("foregroundExternal")));

void InstrumentationThatStartsATransaction()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2020 New Relic, Inc. All rights reserved.
// Copyright 2020 New Relic, Inc. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

using System;
Expand Down Expand Up @@ -366,11 +366,11 @@ private MetricWireModel WaitForMetric()
const int maxLoops = 30;
var loops = 0;
_compositeTestAgent.Harvest();
var cleanupCount = _compositeTestAgent.Metrics.FirstOrDefault(m => m.MetricName.Name == "Supportability/Dotnet/RedisSessionCacheCleanup/Count");
var cleanupCount = _compositeTestAgent.Metrics.FirstOrDefault(m => m.MetricNameModel.Name == "Supportability/Dotnet/RedisSessionCacheCleanup/Count");
while (cleanupCount == null && loops < maxLoops)
{
_compositeTestAgent.Harvest();
cleanupCount = _compositeTestAgent.Metrics.FirstOrDefault(m => m.MetricName.Name == "Supportability/Dotnet/RedisSessionCacheCleanup/Count");
cleanupCount = _compositeTestAgent.Metrics.FirstOrDefault(m => m.MetricNameModel.Name == "Supportability/Dotnet/RedisSessionCacheCleanup/Count");
loops++;
Thread.Sleep(100);
}
Expand Down
Loading