Skip to content

Commit

Permalink
[TEST] Add missing tests to Bazel build (#3045)
Browse files Browse the repository at this point in the history 
* Fix overflow in timeout logic

Also use steady clock consistently.

Prior to this change, the test would fail under ASAN:

bazel test --config=asan --test_output=errors //sdk/test/metrics:meter_provider_sdk_test
INFO: Analyzed target //sdk/test/metrics:meter_provider_sdk_test (0 packages loaded, 0 targets configured).
FAIL: //sdk/test/metrics:meter_provider_sdk_test (see /private/var/tmp/_bazel_punya/e3bd968ba61238cdeb1a5537fc3dbf7d/execroot/_main/bazel-out/darwin_arm64-fastbuild-asan/testlogs/sdk/test/metrics/meter_provider_sdk_test/test.log)
INFO: From Testing //sdk/test/metrics:meter_provider_sdk_test:
==================== Test output for //sdk/test/metrics:meter_provider_sdk_test:
Running main() from gmock_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from MeterProvider
[ RUN      ] MeterProvider.GetMeter
[Warning] File: sdk/src/metrics/meter_provider.cc:65 [MeterProvider::GetMeter] Library name is empty.
[Warning] File: sdk/src/metrics/meter_provider.cc:65 [MeterProvider::GetMeter] Library name is empty.
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__chrono/duration.h:102:59: runtime error: signed integer overflow: 9221646818050376183 * 1000 cannot be represented in type '_Ct' (aka 'long long')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__chrono/duration.h:102:59 in 
================================================================================
INFO: Found 1 test target...
Target //sdk/test/metrics:meter_provider_sdk_test up-to-date:
  bazel-bin/sdk/test/metrics/meter_provider_sdk_test
INFO: Elapsed time: 2.251s, Critical Path: 2.13s
INFO: 5 processes: 5 darwin-sandbox.
INFO: Build completed, 1 test FAILED, 5 total actions
//sdk/test/metrics:meter_provider_sdk_test                               FAILED in 0.6s
  /private/var/tmp/_bazel_punya/e3bd968ba61238cdeb1a5537fc3dbf7d/execroot/_main/bazel-out/darwin_arm64-fastbuild-asan/testlogs/sdk/test/metrics/meter_provider_sdk_test/test.log

Executed 1 out of 1 test: 1 fails locally.

Fix overflow in periodic_exporting_metric_reader

* Add missing tests to Bazel build by globbing test files

Previously, we were missing
* instrument_metadata_validator_test
* observable_registry_test
* cardinality_limit_test
* periodic_exporting_metric_reader_test
And there were no checks in place to prevent things from getting worse.

Remove unnecessary exception checks in attributes_hashmap_test,
which simplifies the build and CI script.

Resolve symbol collision using anonymous namespaces.

---------

Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com>
  • Loading branch information
@punya @lalitb
punya and lalitb authored Sep 3, 2024
1 parent f02d70b commit 7f785b5
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 310 deletions.
4 changes: 2 additions & 2 deletions ci/do_ci.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,8 +484,8 @@ elif [[ "$1" == "bazel.noexcept" ]]; then
# there are some exceptions and error handling code from the Prometheus Client
# as well as Opentracing shim (due to some third party code in its Opentracing dependency)
# that make this test always fail. Ignore these packages in the noexcept test here.
bazel $BAZEL_STARTUP_OPTIONS build --copt=-fno-exceptions $BAZEL_OPTIONS_ASYNC -- //... -//exporters/prometheus/... -//examples/prometheus/... -//sdk/test/metrics:attributes_hashmap_test -//opentracing-shim/...
bazel $BAZEL_STARTUP_OPTIONS test --copt=-fno-exceptions $BAZEL_TEST_OPTIONS_ASYNC -- //... -//exporters/prometheus/... -//examples/prometheus/... -//sdk/test/metrics:attributes_hashmap_test -//opentracing-shim/...
bazel $BAZEL_STARTUP_OPTIONS build --copt=-fno-exceptions $BAZEL_OPTIONS_ASYNC -- //... -//exporters/prometheus/... -//examples/prometheus/... -//opentracing-shim/...
bazel $BAZEL_STARTUP_OPTIONS test --copt=-fno-exceptions $BAZEL_TEST_OPTIONS_ASYNC -- //... -//exporters/prometheus/... -//examples/prometheus/... -//opentracing-shim/...
exit 0
elif [[ "$1" == "bazel.nortti" ]]; then
# there are some exceptions and error handling code from the Prometheus Client
Expand Down
303 changes: 2 additions & 301 deletions sdk/test/metrics/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,307 +19,8 @@ cc_library(
)

cc_test(
name = "meter_test",
srcs = [
"meter_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"metrics_common_test_utils",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "meter_provider_sdk_test",
srcs = [
"meter_provider_sdk_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"metrics_common_test_utils",
"//sdk/src/resource",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "metric_reader_test",
srcs = [
"metric_reader_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"metrics_common_test_utils",
"//sdk/src/resource",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "histogram_test",
srcs = [
"histogram_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"metrics_common_test_utils",
"//sdk/src/resource",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "view_registry_test",
srcs = [
"view_registry_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"metrics_common_test_utils",
"//sdk/src/resource",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "aggregation_test",
srcs = [
"aggregation_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"metrics_common_test_utils",
"//sdk/src/resource",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "sync_metric_storage_counter_test",
srcs = [
"sync_metric_storage_counter_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"metrics_common_test_utils",
"//sdk/src/resource",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "sync_metric_storage_up_down_counter_test",
srcs = [
"sync_metric_storage_up_down_counter_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"metrics_common_test_utils",
"//sdk/src/resource",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "sync_metric_storage_histogram_test",
srcs = [
"sync_metric_storage_histogram_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"metrics_common_test_utils",
"//sdk/src/resource",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "sync_instruments_test",
srcs = [
"sync_instruments_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"metrics_common_test_utils",
"//sdk/src/resource",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "async_instruments_test",
srcs = [
"async_instruments_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"metrics_common_test_utils",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "async_metric_storage_test",
srcs = [
"async_metric_storage_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"metrics_common_test_utils",
"//sdk/src/resource",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "observer_result_test",
srcs = [
"observer_result_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"metrics_common_test_utils",
"//sdk/src/resource",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "multi_metric_storage_test",
srcs = [
"multi_metric_storage_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"metrics_common_test_utils",
"//sdk/src/resource",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "attributes_processor_test",
srcs = [
"attributes_processor_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"metrics_common_test_utils",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "attributes_hashmap_test",
srcs = [
"attributes_hashmap_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"metrics_common_test_utils",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "circular_buffer_counter_test",
srcs = [
"circular_buffer_counter_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"//sdk/src/metrics",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "base2_exponential_histogram_indexer_test",
srcs = [
"base2_exponential_histogram_indexer_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"metrics_common_test_utils",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "histogram_aggregation_test",
srcs = [
"histogram_aggregation_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"metrics_common_test_utils",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "sum_aggregation_test",
srcs = [
"sum_aggregation_test.cc",
],
name = "all_tests",
srcs = glob(["*_test.cc"]),
tags = [
"metrics",
"test",
Expand Down
3 changes: 3 additions & 0 deletions sdk/test/metrics/async_instruments_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@ using namespace opentelemetry::sdk::metrics;

using M = std::map<std::string, std::string>;

namespace
{
// NOLINTNEXTLINE
void asyc_generate_measurements(opentelemetry::metrics::ObserverResult /* observer */,
void * /* state */)
{}
} // namespace

TEST(AsyncInstruments, ObservableInstrument)
{
Expand Down
10 changes: 5 additions & 5 deletions sdk/test/metrics/attributes_hashmap_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ TEST(AttributesHashMap, BasicTests)
std::unique_ptr<Aggregation> aggregation1(
new DropAggregation()); // = std::unique_ptr<Aggregation>(new DropAggregation);
hash_map.Set(m1, std::move(aggregation1), hash);
EXPECT_NO_THROW(hash_map.Get(hash)->Aggregate(static_cast<int64_t>(1)));
hash_map.Get(hash)->Aggregate(static_cast<int64_t>(1));
EXPECT_EQ(hash_map.Size(), 1);
EXPECT_EQ(hash_map.Has(hash), true);

// Set same key again
auto aggregation2 = std::unique_ptr<Aggregation>(new DropAggregation());
hash_map.Set(m1, std::move(aggregation2), hash);
EXPECT_NO_THROW(hash_map.Get(hash)->Aggregate(static_cast<int64_t>(1)));
hash_map.Get(hash)->Aggregate(static_cast<int64_t>(1));
EXPECT_EQ(hash_map.Size(), 1);
EXPECT_EQ(hash_map.Has(hash), true);

Expand All @@ -44,7 +44,7 @@ TEST(AttributesHashMap, BasicTests)
hash_map.Set(m3, std::move(aggregation3), hash3);
EXPECT_EQ(hash_map.Has(hash), true);
EXPECT_EQ(hash_map.Has(hash3), true);
EXPECT_NO_THROW(hash_map.Get(hash3)->Aggregate(static_cast<int64_t>(1)));
hash_map.Get(hash3)->Aggregate(static_cast<int64_t>(1));
EXPECT_EQ(hash_map.Size(), 2);

// GetOrSetDefault
Expand All @@ -54,8 +54,8 @@ TEST(AttributesHashMap, BasicTests)
};
MetricAttributes m4 = {{"k1", "v1"}, {"k2", "v2"}, {"k3", "v3"}};
auto hash4 = opentelemetry::sdk::common::GetHashForAttributeMap(m4);
EXPECT_NO_THROW(hash_map.GetOrSetDefault(m4, create_default_aggregation, hash4)
->Aggregate(static_cast<int64_t>(1)));
hash_map.GetOrSetDefault(m4, create_default_aggregation, hash4)
->Aggregate(static_cast<int64_t>(1));
EXPECT_EQ(hash_map.Size(), 3);

// Set attributes with different order - shouldn't create a new entry.
Expand Down
2 changes: 1 addition & 1 deletion sdk/test/metrics/meter_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@ nostd::shared_ptr<metrics::Meter> InitMeter(MetricReader **metricReaderPtr,
auto meter = provider->GetMeter(meter_name);
return meter;
}
} // namespace

void asyc_generate_measurements(opentelemetry::metrics::ObserverResult observer, void * /* state */)
{
auto observer_long =
nostd::get<nostd::shared_ptr<opentelemetry::metrics::ObserverResultT<int64_t>>>(observer);
observer_long->Observe(10);
}
} // namespace

TEST(MeterTest, BasicAsyncTests)
{
Expand Down
2 changes: 1 addition & 1 deletion sdk/test/metrics/periodic_exporting_metric_reader_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ TEST(PeriodicExporingMetricReader, BasicTests)
MockMetricProducer producer;
reader->SetMetricProducer(&producer);
std::this_thread::sleep_for(std::chrono::milliseconds(2000));
EXPECT_NO_THROW(reader->ForceFlush());
reader->ForceFlush();
reader->Shutdown();
EXPECT_EQ(static_cast<MockPushMetricExporter *>(exporter_ptr)->GetDataCount(),
static_cast<MockMetricProducer *>(&producer)->GetDataCount());
Expand Down

0 comments on commit 7f785b5

Please sign in to comment.