Skip to content

Commit 662c5c5

Browse files
committed
Optimize, linting
1 parent 60f8328 commit 662c5c5

File tree

5 files changed

+62
-68
lines changed

5 files changed

+62
-68
lines changed

agents/otlp/src/otlp_common.cc

Lines changed: 47 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@
1010
#include "nsolid/nsolid_util.h"
1111
#include "nlohmann/json.hpp"
1212
#include "opentelemetry/semconv/incubating/process_attributes.h"
13-
#include "opentelemetry/semconv/incubating/service_attributes.h"
13+
#include "opentelemetry/exporters/otlp/otlp_populate_attribute_utils.h"
1414
#include "opentelemetry/semconv/incubating/thread_attributes.h"
15+
#include "opentelemetry/semconv/incubating/service_attributes.h"
1516
#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h"
1617
#include "opentelemetry/sdk/logs/recordable.h"
1718
#include "opentelemetry/sdk/trace/recordable.h"
@@ -27,7 +28,10 @@ using std::chrono::microseconds;
2728
using std::chrono::milliseconds;
2829
using std::chrono::nanoseconds;
2930

31+
using google::protobuf::RepeatedPtrField;
3032
using opentelemetry::common::SystemTimestamp;
33+
using opentelemetry::proto::common::v1::KeyValue;
34+
using opentelemetry::proto::collector::metrics::v1::ExportMetricsServiceRequest;
3135
using opentelemetry::sdk::instrumentationscope::InstrumentationScope;
3236
using LogsRecordable = opentelemetry::sdk::logs::Recordable;
3337
using opentelemetry::sdk::common::OwnedAttributeType;
@@ -44,6 +48,8 @@ using opentelemetry::sdk::metrics::ValueType;
4448
using opentelemetry::sdk::resource::Resource;
4549
using opentelemetry::sdk::resource::ResourceAttributes;
4650
using opentelemetry::sdk::trace::Recordable;
51+
using OtlpPopulateAttributeUtils =
52+
opentelemetry::exporter::otlp::OtlpPopulateAttributeUtils;
4753

4854
namespace proto = opentelemetry::proto;
4955

@@ -67,41 +73,31 @@ static bool isResourceInitialized_g = false;
6773

6874
// Helper to serialize attributes
6975
void SerializeAttributes(const PointAttributes& attrs,
70-
google::protobuf::RepeatedPtrField<opentelemetry::proto::common::v1::KeyValue>* proto_attrs) {
76+
RepeatedPtrField<KeyValue>* proto_attrs) {
7177
for (const auto& attr : attrs) {
72-
auto* kv = proto_attrs->Add();
73-
kv->set_key(std::string(attr.first));
74-
if (std::holds_alternative<std::string>(attr.second)) {
75-
kv->mutable_value()->set_string_value(std::get<std::string>(attr.second));
76-
} else if (std::holds_alternative<int64_t>(attr.second)) {
77-
kv->mutable_value()->set_int_value(std::get<int64_t>(attr.second));
78-
} else if (std::holds_alternative<uint64_t>(attr.second)) {
79-
kv->mutable_value()->set_int_value(static_cast<int64_t>(std::get<uint64_t>(attr.second)));
80-
} else if (std::holds_alternative<double>(attr.second)) {
81-
kv->mutable_value()->set_double_value(std::get<double>(attr.second));
82-
} else if (std::holds_alternative<bool>(attr.second)) {
83-
kv->mutable_value()->set_bool_value(std::get<bool>(attr.second));
84-
}
78+
OtlpPopulateAttributeUtils::PopulateAttribute(
79+
proto_attrs->Add(), attr.first, attr.second, false);
8580
}
8681
}
8782

88-
opentelemetry::sdk::metrics::MetricData BatchedMetricToMetricData(const BatchedMetricData& bm) {
83+
opentelemetry::sdk::metrics::MetricData BatchedMetricToMetricData(
84+
const BatchedMetricData& bm) {
8985
opentelemetry::sdk::metrics::MetricData md;
9086
md.instrument_descriptor = bm.instrument_descriptor;
9187
md.aggregation_temporality = bm.aggregation_temporality;
9288
if (!bm.point_data_attr_.empty()) {
9389
md.start_ts = bm.point_data_attr_[0].start_ts;
9490
md.end_ts = bm.point_data_attr_[0].end_ts;
9591
for (const auto& p : bm.point_data_attr_) {
96-
opentelemetry::sdk::metrics::PointDataAttributes pda { p.attributes, p.point_data };
97-
md.point_data_attr_.push_back(std::move(pda));
92+
md.point_data_attr_.push_back({ p.attributes, p.point_data });
9893
}
9994
}
10095
return md;
10196
}
10297

10398
// Helper to convert vector<BatchedMetricData> to vector<MetricData>
104-
std::vector<opentelemetry::sdk::metrics::MetricData> ConvertBatchedToMetricData(const std::vector<BatchedMetricData>& batched) {
99+
std::vector<opentelemetry::sdk::metrics::MetricData>
100+
ConvertBatchedToMetricData(const std::vector<BatchedMetricData>& batched) {
105101
std::vector<opentelemetry::sdk::metrics::MetricData> metric_data;
106102
metric_data.reserve(batched.size());
107103
for (const auto& bm : batched) {
@@ -187,7 +183,7 @@ void MetricDataBatch::AddDataPoint(const time_point& start,
187183
InstrumentValueType value_type,
188184
AggregationTemporality temporality,
189185
PointDataAttributes&& pdata_attrs) {
190-
TimedPointDataAttributes timed_point_data_attributes {
186+
TimedPointDataAttributes timed_point_data_attrs {
191187
SystemTimestamp{ start },
192188
SystemTimestamp{ end },
193189
std::move(pdata_attrs.attributes),
@@ -198,12 +194,13 @@ void MetricDataBatch::AddDataPoint(const time_point& start,
198194
BatchedMetricData metric_data {
199195
InstrumentDescriptor { name, "", unit, type, value_type },
200196
temporality,
201-
std::vector<TimedPointDataAttributes>{std::move(timed_point_data_attributes)}
197+
std::vector<TimedPointDataAttributes>{std::move(timed_point_data_attrs)}
202198
};
203199
metrics_.push_back(metric_data);
204200
TrackMetricIndex(name, metrics_.size() - 1);
205201
} else {
206-
metrics_[it->second].point_data_attr_.push_back(std::move(timed_point_data_attributes));
202+
metrics_[it->second].point_data_attr_.
203+
push_back(std::move(timed_point_data_attrs));
207204
}
208205
}
209206

@@ -602,41 +599,38 @@ void fill_recordable(Recordable* recordable, const Tracer::SpanStor& s) {
602599
void PopulateRequest(const std::vector<BatchedMetricData>& metrics,
603600
const Resource* resource,
604601
const InstrumentationScope* scope,
605-
proto::collector::metrics::v1::ExportMetricsServiceRequest* request) {
602+
ExportMetricsServiceRequest* request) {
606603
if (!request) return;
607604

608605
auto* resource_metrics = request->add_resource_metrics();
609606

610607
// Populate resource
611608
if (resource) {
612-
auto* proto_resource = resource_metrics->mutable_resource();
613-
for (const auto& attr : resource->GetAttributes()) {
614-
auto* kv = proto_resource->add_attributes();
615-
kv->set_key(std::string(attr.first));
616-
// Simplified: assume string
617-
kv->mutable_value()->set_string_value(std::get<std::string>(attr.second));
618-
}
609+
OtlpPopulateAttributeUtils::PopulateAttribute(
610+
resource_metrics->mutable_resource(), *resource);
611+
resource_metrics->set_schema_url(resource->GetSchemaURL());
619612
}
620-
resource_metrics->set_schema_url("");
621613

622614
// Scope
623615
auto* scope_metrics = resource_metrics->add_scope_metrics();
624616
if (scope) {
625617
auto* proto_scope = scope_metrics->mutable_scope();
626618
proto_scope->set_name(scope->GetName());
627619
proto_scope->set_version(scope->GetVersion());
620+
OtlpPopulateAttributeUtils::PopulateAttribute(proto_scope, *scope);
621+
scope_metrics->set_schema_url(scope->GetSchemaURL());
628622
}
629-
scope_metrics->set_schema_url("");
630623

631624
// Metrics
632625
for (const auto& batched_metric : metrics) {
633626
auto* proto_metric = scope_metrics->add_metrics();
634627
proto_metric->set_name(batched_metric.instrument_descriptor.name_);
635628
proto_metric->set_unit(batched_metric.instrument_descriptor.unit_);
636629
proto_metric->set_description("");
637-
630+
auto type = batched_metric.instrument_descriptor.type_;
631+
auto value_type = batched_metric.instrument_descriptor.value_type_;
638632
// Type
639-
if (batched_metric.instrument_descriptor.type_ == InstrumentType::kCounter) {
633+
if (type == InstrumentType::kCounter) {
640634
auto* sum = proto_metric->mutable_sum();
641635
sum->set_aggregation_temporality(
642636
static_cast<proto::metrics::v1::AggregationTemporality>(
@@ -649,43 +643,40 @@ void PopulateRequest(const std::vector<BatchedMetricData>& metrics,
649643
// Attributes
650644
SerializeAttributes(point.attributes, dp->mutable_attributes());
651645
// Value
652-
if (const auto* sum_data = std::get_if<SumPointData>(&point.point_data)) {
653-
if (batched_metric.instrument_descriptor.value_type_ == InstrumentValueType::kInt) {
654-
dp->set_as_int(std::get<int64_t>(sum_data->value_));
655-
} else {
656-
dp->set_as_double(std::get<double>(sum_data->value_));
657-
}
646+
auto sum_data = std::get<SumPointData>(point.point_data);
647+
if (value_type == InstrumentValueType::kInt) {
648+
dp->set_as_int(std::get<int64_t>(sum_data.value_));
649+
} else {
650+
dp->set_as_double(std::get<double>(sum_data.value_));
658651
}
659652
}
660-
} else if (batched_metric.instrument_descriptor.type_ == InstrumentType::kGauge) {
653+
} else if (type == InstrumentType::kGauge) {
661654
auto* gauge = proto_metric->mutable_gauge();
662655
for (const auto& point : batched_metric.point_data_attr_) {
663656
auto* dp = gauge->add_data_points();
664657
dp->set_time_unix_nano(point.end_ts.time_since_epoch().count());
665658
SerializeAttributes(point.attributes, dp->mutable_attributes());
666-
if (const auto* gauge_data = std::get_if<LastValuePointData>(&point.point_data)) {
667-
if (batched_metric.instrument_descriptor.value_type_ == InstrumentValueType::kInt) {
668-
dp->set_as_int(std::get<int64_t>(gauge_data->value_));
669-
} else {
670-
dp->set_as_double(std::get<double>(gauge_data->value_));
671-
}
659+
auto gauge_data = std::get<LastValuePointData>(point.point_data);
660+
if (value_type == InstrumentValueType::kInt) {
661+
dp->set_as_int(std::get<int64_t>(gauge_data.value_));
662+
} else {
663+
dp->set_as_double(std::get<double>(gauge_data.value_));
672664
}
673665
}
674-
} else if (batched_metric.instrument_descriptor.type_ == InstrumentType::kSummary) {
666+
} else if (type == InstrumentType::kSummary) {
675667
auto* summary = proto_metric->mutable_summary();
676668
for (const auto& point : batched_metric.point_data_attr_) {
677669
auto* dp = summary->add_data_points();
678670
dp->set_start_time_unix_nano(point.start_ts.time_since_epoch().count());
679671
dp->set_time_unix_nano(point.end_ts.time_since_epoch().count());
680672
SerializeAttributes(point.attributes, dp->mutable_attributes());
681-
if (const auto* summary_data = std::get_if<SummaryPointData>(&point.point_data)) {
682-
dp->set_sum(std::get<double>(summary_data->quantile_values_.at(0.5)));
683-
dp->set_count(1);
684-
for (const auto& q : summary_data->quantile_values_) {
685-
auto* quantile = dp->add_quantile_values();
686-
quantile->set_quantile(q.first);
687-
quantile->set_value(std::get<double>(q.second));
688-
}
673+
auto summary_data = std::get<SummaryPointData>(point.point_data);
674+
dp->set_sum(std::get<double>(summary_data.quantile_values_.at(0.5)));
675+
dp->set_count(1);
676+
for (const auto& q : summary_data.quantile_values_) {
677+
auto* quantile = dp->add_quantile_values();
678+
quantile->set_quantile(q.first);
679+
quantile->set_value(std::get<double>(q.second));
689680
}
690681
}
691682
}

agents/otlp/src/otlp_common.h

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -128,14 +128,17 @@ void fill_log_recordable(OPENTELEMETRY_NAMESPACE::sdk::logs::Recordable*,
128128
void fill_recordable(OPENTELEMETRY_NAMESPACE::sdk::trace::Recordable*,
129129
const Tracer::SpanStor&);
130130

131-
void PopulateRequest(const std::vector<BatchedMetricData>& metrics,
132-
const opentelemetry::sdk::resource::Resource* resource,
133-
const opentelemetry::sdk::instrumentationscope::InstrumentationScope* scope,
134-
opentelemetry::proto::collector::metrics::v1::ExportMetricsServiceRequest* request);
131+
void PopulateRequest(
132+
const std::vector<BatchedMetricData>& metrics,
133+
const opentelemetry::sdk::resource::Resource* resource,
134+
const opentelemetry::sdk::instrumentationscope::InstrumentationScope*,
135+
opentelemetry::proto::collector::metrics::v1::ExportMetricsServiceRequest*);
135136

136-
opentelemetry::sdk::metrics::MetricData BatchedMetricToMetricData(const BatchedMetricData& bm);
137+
opentelemetry::sdk::metrics::MetricData
138+
BatchedMetricToMetricData(const BatchedMetricData& bm);
137139

138-
std::vector<opentelemetry::sdk::metrics::MetricData> ConvertBatchedToMetricData(const std::vector<BatchedMetricData>& batched);
140+
std::vector<opentelemetry::sdk::metrics::MetricData>
141+
ConvertBatchedToMetricData(const std::vector<BatchedMetricData>& batched);
139142

140143
} // namespace otlp
141144
} // namespace nsolid

test/agents/test-otlp-grpc-metrics.mjs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,7 @@ if (process.argv[2] === 'child') {
513513
} else { // worker-thread
514514
assert.strictEqual(dataPoint.attributes[nameIndex].value.stringValue, 'worker-thread');
515515
}
516-
console.log(`Processing thread ${context.threadId}, attributes:`, dataPoint.attributes.map(a => `${a.key}=${a.value.intValue || a.value.stringValue}`).join(', '));
516+
console.log(`Processing thread ${context.threadId}, attributes:`, dataPoint.attributes.map((a) => `${a.key}=${a.value.intValue || a.value.stringValue}`).join(', '));
517517
dataPoints.splice(dataPointIndex, 1);
518518
if (dataPoints.length === 0) {
519519
indicesToRemove.push(i);
@@ -568,7 +568,7 @@ if (process.argv[2] === 'child') {
568568
const threadIdsInBatch = new Set();
569569
metrics.forEach((metric) => {
570570
const aggregation = metric.data;
571-
if (metric[aggregation] && metric[aggregation].dataPoints) {
571+
if (metric[aggregation]?.dataPoints) {
572572
metric[aggregation].dataPoints.forEach((dp) => {
573573
if (dp.attributes) {
574574
const threadIdAttr = dp.attributes.find((a) => a.key === 'thread.id');
@@ -633,7 +633,7 @@ if (process.argv[2] === 'child') {
633633
// Find which thread is in the batch
634634
const threadIdsInBatch = new Set();
635635
context.metrics.forEach((metric) => {
636-
if (metric[metric.data] && metric[metric.data].dataPoints) {
636+
if (metric[metric.data]?.dataPoints) {
637637
metric[metric.data].dataPoints.forEach((dp) => {
638638
if (dp.attributes) {
639639
const threadIdAttr = dp.attributes.find((a) => a.key === 'thread.id');
@@ -664,7 +664,7 @@ if (process.argv[2] === 'child') {
664664
return;
665665
}
666666
context.threadId = foundThread;
667-
context.threadList = context.threadList.filter(t => t !== foundThread);
667+
context.threadList = context.threadList.filter((t) => t !== foundThread);
668668
context.expected = [...expectedThreadMetrics];
669669
} else if (hasProcMetrics) {
670670
context.state = State.ProcMetrics;

test/agents/test-otlp-metrics.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ if (process.argv[2] === 'child') {
560560
const threadIdsInBatch = new Set();
561561
metrics.forEach((metric) => {
562562
const aggregation = metric.data;
563-
if (metric[aggregation] && metric[aggregation].dataPoints) {
563+
if (metric[aggregation]?.dataPoints) {
564564
metric[aggregation].dataPoints.forEach((dp) => {
565565
if (dp.attributes) {
566566
const threadIdAttr = dp.attributes.find((a) => a.key === 'thread.id');

test/common/nsolid-grpc-agent/validators.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ function checkOTLPMetricsData(resourceMetrics,
259259
}
260260

261261
// Ensure timestamps are unique for each datapoint in the metric
262-
const timestamps = new Set(dataPoints.map(dp => dp.timeUnixNano));
262+
const timestamps = new Set(dataPoints.map((dp) => dp.timeUnixNano));
263263
assert.strictEqual(timestamps.size, dataPoints.length, `Timestamps should be unique for process metric ${expectedMetric[0]}`);
264264

265265
const dataPoint = dataPoints[0];

0 commit comments

Comments
 (0)