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.

Sign up for GitHub

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

fix(metrics): ostream exporter should print out resource attributes #1523

Merged
merged 9 commits into from
Aug 5, 2022
Merged

fix(metrics): ostream exporter should print out resource attributes #1523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6a518dc
fix(metrics): ostream exporter should print out resource attributes
bsarden Jul 28, 2022
1eec14e
test(exporters): update ostream_metric_test to check for resource attrs
bsarden Jul 28, 2022
79124bb
chore: fix lint
bsarden Jul 29, 2022
361c13f
fix(test): update ostream_metrics_test to new opentelemetry sdk version
bsarden Jul 31, 2022
ade6377
test(ostream_metrics): use version.h instead of hardcoding sdk version
bsarden Jul 31, 2022
a56d235
Merge branch 'main' into dev/metrics-resources-ostream-exporter
lalitb Aug 1, 2022
af0ee1f
Merge branch 'main' into dev/metrics-resources-ostream-exporter
lalitb Aug 2, 2022
05489f8
feat(metrics): enforce iteration order when printing resource attrs
bsarden Aug 5, 2022
672354b
Merge branch 'main' into dev/metrics-resources-ostream-exporter
lalitb Aug 5, 2022
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
6 changes: 5 additions & 1 deletion exporters/ostream/include/opentelemetry/exporters/ostream/metric_exporter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,13 @@ class OStreamMetricExporter final : public opentelemetry::sdk::metrics::MetricEx
bool is_shutdown_ = false;
mutable opentelemetry::common::SpinLockMutex lock_;
bool isShutdown() const noexcept;
void printInstrumentationInfoMetricData(const sdk::metrics::ScopeMetrics &info_metrics);
void printInstrumentationInfoMetricData(const sdk::metrics::ScopeMetrics &info_metrics,
const sdk::metrics::ResourceMetrics &data);
void printPointData(const opentelemetry::sdk::metrics::PointType &point_data);
void printPointAttributes(const opentelemetry::sdk::metrics::PointAttributes &point_attributes);
void printAttributes(const std::map<std::string, sdk::common::OwnedAttributeValue> &map,
const std::string prefix);
void printResources(const opentelemetry::sdk::resource::Resource &resources);
};
} // namespace metrics
} // namespace exporter
Expand Down
34 changes: 32 additions & 2 deletions exporters/ostream/src/metric_exporter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <chrono>
#ifndef ENABLE_METRICS_PREVIEW
# include <algorithm>
# include <map>
# include "opentelemetry/exporters/ostream/common_utils.h"
# include "opentelemetry/exporters/ostream/metric_exporter.h"
# include "opentelemetry/sdk/metrics/aggregation/default_aggregation.h"
Expand Down Expand Up @@ -79,13 +80,39 @@ sdk::common::ExportResult OStreamMetricExporter::Export(

for (auto &record : data.scope_metric_data_)
{
printInstrumentationInfoMetricData(record);
printInstrumentationInfoMetricData(record, data);
lalitb marked this conversation as resolved.
Show resolved Hide resolved
}
return sdk::common::ExportResult::kSuccess;
}

void OStreamMetricExporter::printAttributes(
const std::map<std::string, sdk::common::OwnedAttributeValue> &map,
const std::string prefix)
{
for (const auto &kv : map)
Copy link
Member

@lalitb lalitb Aug 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bsarden - Looks like this part of the code is causing problem. As it's unordered_map, the order of printing of resource elements is implementation specific (so different in Windows and Linux). Can we ensure to print them in the given order - maybe move resource data to std::map first and then print, and then also ensure that the same order is used for comparison in unit-test? Or else do some smart comparison for resource data in unit-test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bsarden - Just wondering if you have a plan to fix CI as mentioned above? Else, I can do it on top of your repo if you are fine :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing it. CI looks good now :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the ping and reproducing the error on a windows box!

{
sout_ << prefix << kv.first << ": ";
opentelemetry::exporter::ostream_common::print_value(kv.second, sout_);
}
}

void OStreamMetricExporter::printResources(const opentelemetry::sdk::resource::Resource &resources)
{
auto attributes = resources.GetAttributes();
if (attributes.size())
{
// Convert unordered_map to map for printing so that iteration
// order is guaranteed.
std::map<std::string, sdk::common::OwnedAttributeValue> attr_map;
for (auto &kv : attributes)
attr_map[kv.first] = std::move(kv.second);
printAttributes(attr_map, "\n\t");
}
}

void OStreamMetricExporter::printInstrumentationInfoMetricData(
const sdk::metrics::ScopeMetrics &info_metric)
const sdk::metrics::ScopeMetrics &info_metric,
const sdk::metrics::ResourceMetrics &data)
{
// sout_ is shared
const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
Expand All @@ -109,6 +136,9 @@ void OStreamMetricExporter::printInstrumentationInfoMetricData(
printPointAttributes(pd.attributes);
}
}

sout_ << "\n resources\t:";
printResources(*data.resource_);
}
sout_ << "\n}\n";
}
Expand Down
33 changes: 29 additions & 4 deletions exporters/ostream/test/ostream_metric_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
# include <vector>
# include "opentelemetry/sdk/metrics/instruments.h"
# include "opentelemetry/sdk/resource/resource_detector.h"
# include "opentelemetry/sdk/version/version.h"

# include <iostream>
# include "opentelemetry/exporters/ostream/metric_exporter.h"
Expand Down Expand Up @@ -81,7 +82,13 @@ TEST(OStreamMetricsExporter, ExportSumPointData)
"\n value\t\t: 20"
"\n attributes\t\t: "
"\n\ta1: b1"
"\n}\n";
"\n resources\t:"
"\n\tservice.name: unknown_service"
"\n\ttelemetry.sdk.language: cpp"
"\n\ttelemetry.sdk.name: opentelemetry"
"\n\ttelemetry.sdk.version: ";
expected_output += OPENTELEMETRY_SDK_VERSION;
expected_output += "\n}\n";
ASSERT_EQ(stdoutOutput.str(), expected_output);
}

Expand Down Expand Up @@ -151,7 +158,13 @@ TEST(OStreamMetricsExporter, ExportHistogramPointData)
"\n counts : [200, 300, 400, 500, ]"
"\n attributes\t\t: "
"\n\ta1: b1"
"\n}\n";
"\n resources\t:"
"\n\tservice.name: unknown_service"
"\n\ttelemetry.sdk.language: cpp"
"\n\ttelemetry.sdk.name: opentelemetry"
"\n\ttelemetry.sdk.version: ";
expected_output += OPENTELEMETRY_SDK_VERSION;
expected_output += "\n}\n";
ASSERT_EQ(stdoutOutput.str(), expected_output);
}

Expand Down Expand Up @@ -214,7 +227,13 @@ TEST(OStreamMetricsExporter, ExportLastValuePointData)
"\n valid : true"
"\n value : 20"
"\n attributes\t\t: "
"\n}\n";
"\n resources\t:"
"\n\tservice.name: unknown_service"
"\n\ttelemetry.sdk.language: cpp"
"\n\ttelemetry.sdk.name: opentelemetry"
"\n\ttelemetry.sdk.version: ";
expected_output += OPENTELEMETRY_SDK_VERSION;
expected_output += "\n}\n";
ASSERT_EQ(stdoutOutput.str(), expected_output);
}

Expand Down Expand Up @@ -261,7 +280,13 @@ TEST(OStreamMetricsExporter, ExportDropPointData)
"\n name\t\t: library_name"
"\n description\t: description"
"\n unit\t\t: unit"
"\n}\n";
"\n resources\t:"
"\n\tservice.name: unknown_service"
"\n\ttelemetry.sdk.language: cpp"
"\n\ttelemetry.sdk.name: opentelemetry"
"\n\ttelemetry.sdk.version: ";
expected_output += OPENTELEMETRY_SDK_VERSION;
expected_output += "\n}\n";

ASSERT_EQ(stdoutOutput.str(), expected_output);
}
Expand Down