Skip to content

Commit e4cde4e

Browse files
Anilm3dmehala
Anilm3
and
dmehala
authored
Allow setting internal tags and expose numeric tags as metrics (#111)
Allow setting internal tags and expose numeric tags as metrics --------- Co-authored-by: Damien Mehala <[email protected]>
1 parent b553930 commit e4cde4e

File tree

5 files changed

+180
-52
lines changed

5 files changed

+180
-52
lines changed

‎src/datadog/span.cpp‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -101,27 +101,33 @@ const std::string& Span::name() const { return data_->name; }
101101
const std::string& Span::resource_name() const { return data_->resource; }
102102

103103
Optional<StringView> Span::lookup_tag(StringView name) const {
104-
if (tags::is_internal(name)) {
104+
const auto found = data_->tags.find(std::string(name));
105+
if (found == data_->tags.end()) {
105106
return nullopt;
106107
}
108+
return found->second;
109+
}
107110

108-
const auto found = data_->tags.find(std::string(name));
109-
if (found == data_->tags.end()) {
111+
Optional<double> Span::lookup_metric(StringView name) const {
112+
const auto found = data_->numeric_tags.find(std::string(name));
113+
if (found == data_->numeric_tags.end()) {
110114
return nullopt;
111115
}
112116
return found->second;
113117
}
114118

115119
void Span::set_tag(StringView name, StringView value) {
116-
if (!tags::is_internal(name)) {
117-
data_->tags.insert_or_assign(std::string(name), std::string(value));
118-
}
120+
data_->tags.insert_or_assign(std::string(name), std::string(value));
119121
}
120122

121-
void Span::remove_tag(StringView name) {
122-
if (!tags::is_internal(name)) {
123-
data_->tags.erase(std::string(name));
124-
}
123+
void Span::set_metric(StringView name, double value) {
124+
data_->numeric_tags.insert_or_assign(std::string(name), value);
125+
}
126+
127+
void Span::remove_tag(StringView name) { data_->tags.erase(std::string(name)); }
128+
129+
void Span::remove_metric(StringView name) {
130+
data_->numeric_tags.erase(std::string(name));
125131
}
126132

127133
void Span::set_service_name(StringView service) {

‎src/datadog/span.h‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,12 +125,19 @@ class Span {
125125
// Return the value of the tag having the specified `name`, or return null if
126126
// there is no such tag.
127127
Optional<StringView> lookup_tag(StringView name) const;
128+
// Return the value of the metric having the specified `name`, or return null
129+
// if there is no such metric.
130+
Optional<double> lookup_metric(StringView name) const;
128131
// Overwrite the tag having the specified `name` so that it has the specified
129132
// `value`, or create a new tag.
130133
void set_tag(StringView name, StringView value);
134+
// Overwrite the metric having the specified `name` so that it has the
135+
// specified `value`, or create a new metric.
136+
void set_metric(StringView name, double value);
131137
// Delete the tag having the specified `name` if it exists.
132138
void remove_tag(StringView name);
133-
139+
// Delete the metric having the specified `name` if it exists.
140+
void remove_metric(StringView name);
134141
// Set the name of the service associated with this span, e.g.
135142
// "ingress-nginx-useast1".
136143
void set_service_name(StringView);

‎src/datadog/span_data.cpp‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,7 @@ void SpanData::apply_config(const SpanDefaults& defaults,
5151
tags.insert_or_assign(tags::version, version);
5252
}
5353
for (const auto& [key, value] : config.tags) {
54-
if (!tags::is_internal(key)) {
55-
tags.insert_or_assign(key, value);
56-
}
54+
tags.insert_or_assign(key, value);
5755
}
5856

5957
resource = config.resource.value_or(name);
@@ -139,4 +137,4 @@ Expected<void> msgpack_encode(
139137
}
140138

141139
} // namespace tracing
142-
} // namespace datadog
140+
} // namespace datadog

‎test/system-tests/request_handler.cpp‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,24 @@ void RequestHandler::on_set_meta(const httplib::Request& req,
165165

166166
void RequestHandler::on_set_metric(const httplib::Request& /* req */,
167167
httplib::Response& res) {
168-
// No method available for directly setting a span metric.
169-
// Returning OK instead of UNIMPLEMENTED to satisfy the test framework.
168+
const auto request_json = nlohmann::json::parse(res.body);
169+
170+
auto span_id = utils::get_if_exists<uint64_t>(request_json, "span_id");
171+
if (!span_id) {
172+
VALIDATION_ERROR(res, "on_set_meta: missing `span_id` field.");
173+
}
174+
175+
auto span_it = active_spans_.find(*span_id);
176+
if (span_it == active_spans_.cend()) {
177+
const auto msg =
178+
"on_set_meta: span not found for id " + std::to_string(*span_id);
179+
VALIDATION_ERROR(res, msg);
180+
}
181+
182+
auto& span = span_it->second;
183+
span.set_metric(request_json.at("key").get<std::string_view>(),
184+
request_json.at("value").get<double>());
185+
170186
res.status = 200;
171187
}
172188

‎test/test_span.cpp‎

Lines changed: 136 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ TEST_CASE("set_tag") {
4646
span.set_tag("foo", "lemon");
4747
span.set_tag("foo.bar", "mint");
4848
span.set_tag("foo.baz", "blueberry");
49+
span.set_tag("_dd.secret.sauce", "thousand islands");
50+
span.set_tag("_dd_not_internal", "");
51+
span.set_tag("_dd.chipmunk", "");
4952
}
5053

5154
REQUIRE(collector->chunks.size() == 1);
@@ -57,18 +60,21 @@ TEST_CASE("set_tag") {
5760
REQUIRE(span.tags.at("foo") == "lemon");
5861
REQUIRE(span.tags.at("foo.bar") == "mint");
5962
REQUIRE(span.tags.at("foo.baz") == "blueberry");
63+
REQUIRE(span.tags.at("_dd.secret.sauce") == "thousand islands");
64+
REQUIRE(span.tags.at("_dd_not_internal") == "");
65+
REQUIRE(span.tags.at("_dd.chipmunk") == "");
6066
}
6167

6268
SECTION("tags can be overwritten") {
6369
{
6470
SpanConfig config;
65-
config.tags = {
66-
{"color", "purple"},
67-
{"turtle.depth", "all the way down"},
68-
};
71+
config.tags = {{"color", "purple"},
72+
{"turtle.depth", "all the way down"},
73+
{"_dd.tag", "written"}};
6974
auto span = tracer.create_span(config);
7075
span.set_tag("color", "green");
7176
span.set_tag("bonus", "applied");
77+
span.set_tag("_dd.tag", "overwritten");
7278
}
7379

7480
REQUIRE(collector->chunks.size() == 1);
@@ -80,29 +86,7 @@ TEST_CASE("set_tag") {
8086
REQUIRE(span.tags.at("color") == "green");
8187
REQUIRE(span.tags.at("turtle.depth") == "all the way down");
8288
REQUIRE(span.tags.at("bonus") == "applied");
83-
}
84-
85-
SECTION("can't set internal tags directly") {
86-
{
87-
auto span = tracer.create_span();
88-
span.set_tag("foo", "lemon");
89-
span.set_tag("_dd.secret.sauce", "thousand islands");
90-
span.set_tag("_dd_not_internal", "");
91-
// _dd.p.dm will end up in the tags due to how sampling works
92-
// span.set_tag("_dd.p.dm", "-4");
93-
span.set_tag("_dd.chipmunk", "");
94-
}
95-
96-
REQUIRE(collector->chunks.size() == 1);
97-
const auto& chunk = collector->chunks.front();
98-
REQUIRE(chunk.size() == 1);
99-
const auto& span_ptr = chunk.front();
100-
REQUIRE(span_ptr);
101-
const auto& span = *span_ptr;
102-
REQUIRE(span.tags.at("foo") == "lemon");
103-
REQUIRE(span.tags.count("_dd.secret.sauce") == 0);
104-
REQUIRE(span.tags.at("_dd_not_internal") == "");
105-
REQUIRE(span.tags.count("_dd.chipmunk") == 0);
89+
REQUIRE(span.tags.at("_dd.tag") == "overwritten");
10690
}
10791
}
10892

@@ -120,34 +104,32 @@ TEST_CASE("lookup_tag") {
120104
auto span = tracer.create_span();
121105
REQUIRE(!span.lookup_tag("nope"));
122106
REQUIRE(!span.lookup_tag("also nope"));
107+
REQUIRE(!span.lookup_tag("_dd.nope"));
123108
}
124109

125110
SECTION("lookup after set") {
126111
auto span = tracer.create_span();
127112
span.set_tag("color", "purple");
128113
span.set_tag("turtle.depth", "all the way down");
114+
span.set_tag("_dd.tag", "found");
129115

130116
REQUIRE(span.lookup_tag("color") == "purple");
131117
REQUIRE(span.lookup_tag("turtle.depth") == "all the way down");
118+
REQUIRE(span.lookup_tag("_dd.tag") == "found");
132119
}
133120

134121
SECTION("lookup after config") {
135122
SpanConfig config;
136123
config.tags = {
137124
{"color", "purple"},
138125
{"turtle.depth", "all the way down"},
126+
{"_dd.tag", "found"},
139127
};
140128
auto span = tracer.create_span(config);
141129

142130
REQUIRE(span.lookup_tag("color") == "purple");
143131
REQUIRE(span.lookup_tag("turtle.depth") == "all the way down");
144-
}
145-
146-
SECTION("internal tags redacted") {
147-
auto span = tracer.create_span();
148-
REQUIRE(!span.lookup_tag("_dd.this"));
149-
REQUIRE(!span.lookup_tag("_dd.that"));
150-
REQUIRE(!span.lookup_tag("_dd.the.other.thing"));
132+
REQUIRE(span.lookup_tag("_dd.tag") == "found");
151133
}
152134
}
153135

@@ -164,22 +146,141 @@ TEST_CASE("remove_tag") {
164146
SECTION("doesn't have to be there already") {
165147
auto span = tracer.create_span();
166148
span.remove_tag("not even there");
149+
span.remove_tag("_dd.tag");
167150
}
168151

169152
SECTION("after removal, lookup yields null") {
170153
SpanConfig config;
171-
config.tags = {{"mayfly", "carpe diem"}};
154+
config.tags = {{"mayfly", "carpe diem"}, {"_dd.mayfly", "carpe diem"}};
172155
auto span = tracer.create_span(config);
173156
span.set_tag("foo", "bar");
174157

175158
span.remove_tag("mayfly");
159+
span.remove_tag("_dd.mayfly");
176160
span.remove_tag("foo");
177161

178162
REQUIRE(!span.lookup_tag("mayfly"));
163+
REQUIRE(!span.lookup_tag("_dd.mayfly"));
179164
REQUIRE(!span.lookup_tag("foo"));
180165
}
181166
}
182167

168+
TEST_CASE("set_metric") {
169+
TracerConfig config;
170+
config.service = "testsvc";
171+
const auto collector = std::make_shared<MockCollector>();
172+
config.collector = collector;
173+
config.logger = std::make_shared<MockLogger>();
174+
175+
auto finalized_config = finalize_config(config);
176+
REQUIRE(finalized_config);
177+
Tracer tracer{*finalized_config};
178+
179+
SECTION("metrics end up in the collector") {
180+
{
181+
auto span = tracer.create_span();
182+
span.set_metric("foo", 5.0);
183+
span.set_metric("foo.bar", 3.0);
184+
span.set_metric("foo.baz", 1.0);
185+
span.set_metric("_dd.secret.sauce", 2.0);
186+
span.set_metric("_dd_not_internal", 3.0);
187+
span.set_metric("_dd.chipmunk", 4.0);
188+
}
189+
190+
REQUIRE(collector->chunks.size() == 1);
191+
const auto& chunk = collector->chunks.front();
192+
REQUIRE(chunk.size() == 1);
193+
const auto& span_ptr = chunk.front();
194+
REQUIRE(span_ptr);
195+
const auto& span = *span_ptr;
196+
REQUIRE(span.numeric_tags.at("foo") == 5.0);
197+
REQUIRE(span.numeric_tags.at("foo.bar") == 3.0);
198+
REQUIRE(span.numeric_tags.at("foo.baz") == 1.0);
199+
REQUIRE(span.numeric_tags.at("_dd.secret.sauce") == 2.0);
200+
REQUIRE(span.numeric_tags.at("_dd_not_internal") == 3.0);
201+
REQUIRE(span.numeric_tags.at("_dd.chipmunk") == 4.0);
202+
}
203+
204+
SECTION("metrics can be overwritten") {
205+
{
206+
auto span = tracer.create_span();
207+
span.set_metric("color", 2.0);
208+
span.set_metric("color", 1.0);
209+
span.set_metric("bonus", 6.0);
210+
span.set_metric("bonus", 5.0);
211+
}
212+
213+
REQUIRE(collector->chunks.size() == 1);
214+
const auto& chunk = collector->chunks.front();
215+
REQUIRE(chunk.size() == 1);
216+
const auto& span_ptr = chunk.front();
217+
REQUIRE(span_ptr);
218+
const auto& span = *span_ptr;
219+
REQUIRE(span.numeric_tags.at("color") == 1.0);
220+
REQUIRE(span.numeric_tags.at("bonus") == 5.0);
221+
}
222+
}
223+
224+
TEST_CASE("lookup_metric") {
225+
TracerConfig config;
226+
config.service = "testsvc";
227+
config.collector = std::make_shared<MockCollector>();
228+
config.logger = std::make_shared<MockLogger>();
229+
230+
auto finalized_config = finalize_config(config);
231+
REQUIRE(finalized_config);
232+
Tracer tracer{*finalized_config};
233+
234+
SECTION("not found is null") {
235+
auto span = tracer.create_span();
236+
REQUIRE(!span.lookup_metric("nope"));
237+
REQUIRE(!span.lookup_metric("also nope"));
238+
REQUIRE(!span.lookup_metric("_dd.nope"));
239+
}
240+
241+
SECTION("lookup after set") {
242+
auto span = tracer.create_span();
243+
span.set_metric("color", 11.0);
244+
span.set_metric("turtle.depth", 6.0);
245+
span.set_metric("_dd.this", 33.0);
246+
247+
REQUIRE(span.lookup_metric("color") == 11.0);
248+
REQUIRE(span.lookup_metric("turtle.depth") == 6.0);
249+
REQUIRE(span.lookup_metric("_dd.this") == 33.0);
250+
}
251+
}
252+
253+
TEST_CASE("remove_metric") {
254+
TracerConfig config;
255+
config.service = "testsvc";
256+
config.collector = std::make_shared<MockCollector>();
257+
config.logger = std::make_shared<MockLogger>();
258+
259+
auto finalized_config = finalize_config(config);
260+
REQUIRE(finalized_config);
261+
Tracer tracer{*finalized_config};
262+
263+
SECTION("doesn't have to be there already") {
264+
auto span = tracer.create_span();
265+
span.remove_metric("not even there");
266+
}
267+
268+
SECTION("after removal, lookup yields null") {
269+
auto span = tracer.create_span();
270+
span.set_metric("mayfly", 10.0);
271+
span.set_metric("foo", 11.0);
272+
span.set_metric("_dd.metric", 1.0);
273+
274+
span.remove_metric("mayfly");
275+
span.remove_metric("foo");
276+
span.remove_metric("_dd.metric");
277+
278+
REQUIRE(!span.lookup_metric("mayfly"));
279+
REQUIRE(!span.lookup_metric("foo"));
280+
REQUIRE(!span.lookup_metric("_dd.metric"));
281+
}
282+
}
283+
183284
TEST_CASE("span duration") {
184285
TracerConfig config;
185286
config.service = "testsvc";

0 commit comments

Comments
 (0)