Skip to content

Commit e84e7a5

Browse files
gleocadiegleocadie
authored
[Profiler] Little label improvement (#6830)
## Summary of changes Use only one container in `Sample` class to store labels. ## Reason for change There is 2 types of labels: numeric and string. The type is related to the type of the value. Before that we had 2 containers (one per type of label). ## Implementation details - Use `std::variant` to store `NumericLabel` and `StringLabel` labels in the same container. - Use `std::visit` to have a processing per label type. - Rename `Label` into `StringLabel` - Remove `SpanLabel` and use `NumericLabel` instead (since it's a Numeric label) ## Test coverage Current test should be suffice ## Other details <!-- Fixes #{issue} --> <!-- ⚠️ Note: where possible, please obtain 2 approvals prior to merging. Unless CODEOWNERS specifies otherwise, for external teams it is typically best to have one review from a team member, and one review from apm-dotnet. Trivial changes do not require 2 reviews. -->
1 parent e8b912b commit e84e7a5

18 files changed

+129
-139
lines changed

‎profiler/src/ProfilerEngine/Datadog.Profiler.Native/CollectorBase.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,8 @@ class CollectorBase
9595

9696
if (rawSample.LocalRootSpanId != 0 && rawSample.SpanId != 0)
9797
{
98-
sample->AddNumericLabel(SpanLabel{Sample::LocalRootSpanIdLabel, rawSample.LocalRootSpanId});
99-
sample->AddNumericLabel(SpanLabel{Sample::SpanIdLabel, rawSample.SpanId});
98+
sample->AddLabel(NumericLabel{Sample::LocalRootSpanIdLabel, rawSample.LocalRootSpanId});
99+
sample->AddLabel(NumericLabel{Sample::SpanIdLabel, rawSample.SpanId});
100100
}
101101

102102
// compute thread/appdomain details

‎profiler/src/ProfilerEngine/Datadog.Profiler.Native/GCBaseRawSample.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ class GCBaseRawSample : public RawSample
4242

4343
sample->AddValue(GetValue(), durationIndex);
4444

45-
sample->AddNumericLabel(NumericLabel(Sample::GarbageCollectionNumberLabel, Number));
45+
sample->AddLabel(NumericLabel(Sample::GarbageCollectionNumberLabel, Number));
4646
AddGenerationLabel(sample, Generation);
4747

4848
BuildCallStack(sample, Generation);
@@ -74,19 +74,19 @@ class GCBaseRawSample : public RawSample
7474
switch (generation)
7575
{
7676
case 0:
77-
sample->AddLabel(Label(Sample::GarbageCollectionGenerationLabel, Gen0Value));
77+
sample->AddLabel(StringLabel(Sample::GarbageCollectionGenerationLabel, Gen0Value));
7878
break;
7979

8080
case 1:
81-
sample->AddLabel(Label(Sample::GarbageCollectionGenerationLabel, Gen1Value));
81+
sample->AddLabel(StringLabel(Sample::GarbageCollectionGenerationLabel, Gen1Value));
8282
break;
8383

8484
case 2:
85-
sample->AddLabel(Label(Sample::GarbageCollectionGenerationLabel, Gen2Value));
85+
sample->AddLabel(StringLabel(Sample::GarbageCollectionGenerationLabel, Gen2Value));
8686
break;
8787

8888
default: // this should never happen (only gen0, gen1 or gen2 collections)
89-
sample->AddLabel(Label(Sample::GarbageCollectionGenerationLabel, std::to_string(generation)));
89+
sample->AddLabel(StringLabel(Sample::GarbageCollectionGenerationLabel, std::to_string(generation)));
9090
break;
9191
}
9292
}

‎profiler/src/ProfilerEngine/Datadog.Profiler.Native/GCThreadsCpuProvider.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ std::vector<std::shared_ptr<IThreadInfo>> const& GCThreadsCpuProvider::GetThread
6161

6262
Labels GCThreadsCpuProvider::GetLabels()
6363
{
64-
return Labels{Label{"gc_cpu_sample", "true"}};
64+
return Labels{StringLabel{"gc_cpu_sample", "true"}};
6565
}
6666

6767
std::vector<FrameInfoView> GCThreadsCpuProvider::GetFrames()

‎profiler/src/ProfilerEngine/Datadog.Profiler.Native/LiveObjectInfo.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ LiveObjectInfo::LiveObjectInfo(std::shared_ptr<Sample> sample, uintptr_t address
1414
_gcCount(0)
1515
{
1616
auto id = s_nextObjectId++;
17-
sample->AddLabel(Label{Sample::ObjectIdLabel, std::to_string(id)});
17+
sample->AddLabel(StringLabel{Sample::ObjectIdLabel, std::to_string(id)});
1818

19-
sample->AddLabel(Label{Sample::ObjectGenerationLabel, std::to_string(0)});
20-
sample->AddLabel(Label{Sample::ObjectLifetimeLabel, std::to_string(0)});
19+
sample->AddLabel(StringLabel{Sample::ObjectGenerationLabel, std::to_string(0)});
20+
sample->AddLabel(StringLabel{Sample::ObjectLifetimeLabel, std::to_string(0)});
2121
_sample = sample;
2222
}
2323

‎profiler/src/ProfilerEngine/Datadog.Profiler.Native/LiveObjectsProvider.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,8 @@ std::unique_ptr<SamplesEnumerator> LiveObjectsProvider::GetSamples()
152152
auto sample = info.GetSample();
153153

154154
// update samples lifetime
155-
sample->ReplaceLabel(Label{Sample::ObjectLifetimeLabel, std::to_string((sample->GetTimeStamp() - currentTimestamp).count())});
156-
sample->ReplaceLabel(Label{Sample::ObjectGenerationLabel, info.IsGen2() ? Gen2 : Gen1});
155+
sample->ReplaceLabel(StringLabel{Sample::ObjectLifetimeLabel, std::to_string((sample->GetTimeStamp() - currentTimestamp).count())});
156+
sample->ReplaceLabel(StringLabel{Sample::ObjectGenerationLabel, info.IsGen2() ? Gen2 : Gen1});
157157

158158
samples->Add(sample);
159159
}

‎profiler/src/ProfilerEngine/Datadog.Profiler.Native/NativeThreadsCpuProviderBase.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,9 @@ std::unique_ptr<SamplesEnumerator> NativeThreadsCpuProviderBase::GetSamples()
9696
sample->AddFrame(frame);
9797
}
9898

99-
for (auto const& label : GetLabels())
99+
for (auto&& label : GetLabels())
100100
{
101-
sample->AddLabel(Label{label.first,label.second});
101+
sample->AddLabel(std::move(label));
102102
}
103103

104104
enumerator->Set(sample);

‎profiler/src/ProfilerEngine/Datadog.Profiler.Native/Profile.cpp

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -60,26 +60,29 @@ libdatadog::Success Profile::Add(std::shared_ptr<Sample> const& sample)
6060

6161
// Labels
6262
auto const& labels = sample->GetLabels();
63-
auto const& numericLabels = sample->GetNumericLabels();
6463
std::vector<ddog_prof_Label> ffiLabels;
65-
ffiLabels.reserve(labels.size() + numericLabels.size());
64+
ffiLabels.reserve(labels.size());
6665

67-
for (auto const& [label, value] : labels)
66+
for (auto const& label : labels)
6867
{
69-
auto labelz = ddog_prof_Label {
70-
.key = {label.data(), label.size()},
71-
.str = {value.data(), value.size()}
72-
};
73-
ffiLabels.push_back(std::move(labelz));
74-
}
75-
76-
for (auto const& [label, value] : numericLabels)
77-
{
78-
auto labelz = ddog_prof_Label {
79-
.key = {label.data(), label.size()},
80-
.num = value
81-
};
82-
ffiLabels.push_back(std::move(labelz));
68+
auto ffiLabel = std::visit(
69+
LabelsVisitor{
70+
[](NumericLabel const& l) -> ddog_prof_Label {
71+
auto const& [name, value] = l;
72+
return ddog_prof_Label {
73+
.key = {name.data(), name.size()},
74+
.num = value
75+
};
76+
},
77+
[](StringLabel const& l) -> ddog_prof_Label {
78+
auto const& [name, value] = l;
79+
return ddog_prof_Label {
80+
.key = {name.data(), name.size()},
81+
.str = {value.data(), value.size()}
82+
};
83+
}
84+
}, label);
85+
ffiLabels.push_back(ffiLabel);
8386
}
8487

8588
ffiSample.labels = {ffiLabels.data(), ffiLabels.size()};

‎profiler/src/ProfilerEngine/Datadog.Profiler.Native/RawAllocationSample.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ class RawAllocationSample : public RawSample
4646
sample->AddValue(AllocationSize, allocationSizeIndex);
4747
}
4848

49-
sample->AddLabel(Label(Sample::AllocationClassLabel, AllocationClass));
49+
sample->AddLabel(StringLabel(Sample::AllocationClassLabel, AllocationClass));
5050
}
5151

5252
std::string AllocationClass;

‎profiler/src/ProfilerEngine/Datadog.Profiler.Native/RawContentionSample.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,15 @@ class RawContentionSample : public RawSample
4747
auto contentionCountIndex = valueOffsets[0];
4848
auto contentionDurationIndex = valueOffsets[1];
4949

50-
sample->AddLabel(Label{BucketLabelName, std::move(Bucket)});
50+
sample->AddLabel(StringLabel{BucketLabelName, std::move(Bucket)});
5151
sample->AddValue(1, contentionCountIndex);
52-
sample->AddNumericLabel(NumericLabel{RawCountLabelName, 1});
53-
sample->AddNumericLabel(NumericLabel{RawDurationLabelName, ContentionDuration.count()});
52+
sample->AddLabel(NumericLabel{RawCountLabelName, 1});
53+
sample->AddLabel(NumericLabel{RawDurationLabelName, ContentionDuration.count()});
5454
sample->AddValue(ContentionDuration.count(), contentionDurationIndex);
5555
if (BlockingThreadId != 0)
5656
{
57-
sample->AddNumericLabel(NumericLabel{BlockingThreadIdLabelName, BlockingThreadId});
58-
sample->AddLabel(Label{BlockingThreadNameLabelName, shared::ToString(BlockingThreadName)});
57+
sample->AddLabel(NumericLabel{BlockingThreadIdLabelName, BlockingThreadId});
58+
sample->AddLabel(StringLabel{BlockingThreadNameLabelName, shared::ToString(BlockingThreadName)});
5959
}
6060
}
6161

‎profiler/src/ProfilerEngine/Datadog.Profiler.Native/RawExceptionSample.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ class RawExceptionSample : public RawSample
3030
{
3131
assert(valueOffsets.size() == 1);
3232
sample->AddValue(1, valueOffsets[0]);
33-
sample->AddLabel(Label(Sample::ExceptionMessageLabel, ExceptionMessage));
34-
sample->AddLabel(Label(Sample::ExceptionTypeLabel, ExceptionType));
33+
sample->AddLabel(StringLabel(Sample::ExceptionMessageLabel, ExceptionMessage));
34+
sample->AddLabel(StringLabel(Sample::ExceptionTypeLabel, ExceptionType));
3535
}
3636

3737
std::string ExceptionMessage;

‎profiler/src/ProfilerEngine/Datadog.Profiler.Native/RawGarbageCollectionSample.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,12 @@ class RawGarbageCollectionSample : public GCBaseRawSample
4848

4949
inline void DoAdditionalTransform(std::shared_ptr<Sample> sample, std::vector<SampleValueTypeProvider::Offset> const& valueOffsets) const override
5050
{
51-
sample->AddLabel(Label(Sample::GarbageCollectionReasonLabel, GetReasonText()));
52-
sample->AddLabel(Label(Sample::GarbageCollectionTypeLabel, GetTypeText()));
53-
sample->AddLabel(Label(Sample::GarbageCollectionCompactingLabel, (IsCompacting ? "true" : "false")));
51+
sample->AddLabel(StringLabel(Sample::GarbageCollectionReasonLabel, GetReasonText()));
52+
sample->AddLabel(StringLabel(Sample::GarbageCollectionTypeLabel, GetTypeText()));
53+
sample->AddLabel(StringLabel(Sample::GarbageCollectionCompactingLabel, (IsCompacting ? "true" : "false")));
5454

5555
// set event type
56-
sample->AddLabel(Label(Sample::TimelineEventTypeLabel, Sample::TimelineEventTypeGarbageCollection));
56+
sample->AddLabel(StringLabel(Sample::TimelineEventTypeLabel, Sample::TimelineEventTypeGarbageCollection));
5757
}
5858

5959
public:

‎profiler/src/ProfilerEngine/Datadog.Profiler.Native/RawNetworkSample.h

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -69,45 +69,45 @@ class RawNetworkSample : public RawSample
6969
// Note: we don't need to add the start timestamp as a label because it is computed
7070
// by the backend from the end timestamp and the duration; i.e. the value of this sample
7171

72-
sample->AddLabel(Label(Sample::RequestUrlLabel, Url));
73-
sample->AddNumericLabel(NumericLabel(Sample::RequestStatusCodeLabel, StatusCode));
72+
sample->AddLabel(StringLabel(Sample::RequestUrlLabel, Url));
73+
sample->AddLabel(NumericLabel(Sample::RequestStatusCodeLabel, StatusCode));
7474
if (!Error.empty())
7575
{
76-
sample->AddLabel(Label(Sample::RequestErrorLabel, Error));
76+
sample->AddLabel(StringLabel(Sample::RequestErrorLabel, Error));
7777
}
7878
if (HasBeenRedirected)
7979
{
80-
sample->AddLabel(Label(Sample::RequestRedirectUrlLabel, RedirectUrl));
80+
sample->AddLabel(StringLabel(Sample::RequestRedirectUrlLabel, RedirectUrl));
8181
}
8282
if (DnsDuration != std::chrono::nanoseconds::zero())
8383
{
84-
sample->AddNumericLabel(NumericLabel(Sample::RequestDnsWaitLabel, DnsWait.count()));
85-
sample->AddNumericLabel(NumericLabel(Sample::RequestDnsDurationLabel, DnsDuration.count()));
86-
sample->AddLabel(Label(Sample::RequestDnsSuccessLabel, DnsSuccess ? "true" : "false"));
84+
sample->AddLabel(NumericLabel(Sample::RequestDnsWaitLabel, DnsWait.count()));
85+
sample->AddLabel(NumericLabel(Sample::RequestDnsDurationLabel, DnsDuration.count()));
86+
sample->AddLabel(StringLabel(Sample::RequestDnsSuccessLabel, DnsSuccess ? "true" : "false"));
8787
}
8888
if (HandshakeDuration != std::chrono::nanoseconds::zero())
8989
{
90-
sample->AddNumericLabel(NumericLabel(Sample::RequestHandshakeWaitLabel, HandshakeWait.count()));
91-
sample->AddNumericLabel(NumericLabel(Sample::RequestHandshakeDurationLabel, HandshakeDuration.count()));
90+
sample->AddLabel(NumericLabel(Sample::RequestHandshakeWaitLabel, HandshakeWait.count()));
91+
sample->AddLabel(NumericLabel(Sample::RequestHandshakeDurationLabel, HandshakeDuration.count()));
9292
}
9393
if (!HandshakeError.empty())
9494
{
95-
sample->AddLabel(Label(Sample::RequestHandshakeErrorLabel, HandshakeError));
95+
sample->AddLabel(StringLabel(Sample::RequestHandshakeErrorLabel, HandshakeError));
9696
}
9797
if (SocketConnectDuration != std::chrono::nanoseconds::zero())
9898
{
99-
sample->AddNumericLabel(NumericLabel(Sample::RequestSocketDurationLabel, SocketConnectDuration.count()));
99+
sample->AddLabel(NumericLabel(Sample::RequestSocketDurationLabel, SocketConnectDuration.count()));
100100
}
101101
if (RequestDuration != std::chrono::nanoseconds::zero()) // could be 0 in case of connection/handshake error
102102
{
103-
sample->AddNumericLabel(NumericLabel(Sample::RequestDurationLabel, RequestDuration.count()));
103+
sample->AddLabel(NumericLabel(Sample::RequestDurationLabel, RequestDuration.count()));
104104
}
105105
if (ResponseDuration != std::chrono::nanoseconds::zero()) // could be 0 in case of error
106106
{
107-
sample->AddNumericLabel(NumericLabel(Sample::ResponseContentDurationLabel, ResponseDuration.count()));
107+
sample->AddLabel(NumericLabel(Sample::ResponseContentDurationLabel, ResponseDuration.count()));
108108
}
109-
sample->AddLabel(Label(Sample::RequestResponseThreadIdLabel, EndThreadId));
110-
sample->AddLabel(Label(Sample::RequestResponseThreadNameLabel, EndThreadName));
109+
sample->AddLabel(StringLabel(Sample::RequestResponseThreadIdLabel, EndThreadId));
110+
sample->AddLabel(StringLabel(Sample::RequestResponseThreadNameLabel, EndThreadName));
111111
}
112112

113113
std::string Url;

‎profiler/src/ProfilerEngine/Datadog.Profiler.Native/RawStopTheWorldSample.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,6 @@ class RawStopTheWorldSample : public GCBaseRawSample
1919
void DoAdditionalTransform(std::shared_ptr<Sample> sample, std::vector<SampleValueTypeProvider::Offset> const& valueOffsets) const override
2020
{
2121
// set event type
22-
sample->AddLabel(Label(Sample::TimelineEventTypeLabel, Sample::TimelineEventTypeStopTheWorld));
22+
sample->AddLabel(StringLabel(Sample::TimelineEventTypeLabel, Sample::TimelineEventTypeStopTheWorld));
2323
}
2424
};

‎profiler/src/ProfilerEngine/Datadog.Profiler.Native/RawThreadLifetimeSample.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,12 @@ void RawThreadLifetimeSample::OnTransform(
1313
if (Kind == ThreadEventKind::Start)
1414
{
1515
sample->AddFrame({EmptyModule, StartFrame, "", 0});
16-
sample->AddLabel(Label(Sample::TimelineEventTypeLabel, Sample::TimelineEventTypeThreadStart));
16+
sample->AddLabel(StringLabel(Sample::TimelineEventTypeLabel, Sample::TimelineEventTypeThreadStart));
1717
}
1818
else if (Kind == ThreadEventKind::Stop)
1919
{
2020
sample->AddFrame({EmptyModule, StopFrame, "", 0});
21-
sample->AddLabel(Label(Sample::TimelineEventTypeLabel, Sample::TimelineEventTypeThreadStop));
21+
sample->AddLabel(StringLabel(Sample::TimelineEventTypeLabel, Sample::TimelineEventTypeThreadStop));
2222
}
2323

2424
// Set an arbitratry value to avoid being discarded by the backend

‎profiler/src/ProfilerEngine/Datadog.Profiler.Native/Sample.cpp

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,13 @@ Sample::Sample(std::chrono::nanoseconds timestamp, std::string_view runtimeId, s
5858
_timestamp = timestamp;
5959
_runtimeId = runtimeId;
6060
_callstack.reserve(framesCount);
61-
_labels.reserve(10);
62-
_numericLabels.reserve(10);
61+
_allLabels.reserve(10);
6362
}
6463

6564
Sample::Sample(std::string_view runtimeId) :
6665
_values(ValuesCount),
6766
_timestamp{0},
68-
_labels{},
69-
_numericLabels{},
67+
_allLabels{},
7068
_callstack{},
7169
_runtimeId{runtimeId}
7270
{
@@ -122,10 +120,5 @@ std::string_view Sample::GetRuntimeId() const
122120

123121
const Labels& Sample::GetLabels() const
124122
{
125-
return _labels;
126-
}
127-
128-
const NumericLabels& Sample::GetNumericLabels() const
129-
{
130-
return _numericLabels;
123+
return _allLabels;
131124
}

0 commit comments

Comments
 (0)