From 227a577dd96e528fcdf842ef4dc2eb72bd5d4a6b Mon Sep 17 00:00:00 2001 From: Alex E Date: Sun, 22 Dec 2024 13:36:30 -0500 Subject: [PATCH 01/16] Add environment-related logic --- .../exporters/otlp/otlp_environment.h | 18 +++ exporters/otlp/src/otlp_environment.cc | 126 ++++++++++++++++++ 2 files changed, 144 insertions(+) diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h index 74a222a9d0..799e07f27d 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h @@ -18,6 +18,8 @@ namespace exporter namespace otlp { +using SecondsDecimal = std::chrono::duration>; + inline std::string GetOtlpDefaultUserAgent() { return "OTel-OTLP-Exporter-Cpp/" OPENTELEMETRY_SDK_VERSION; @@ -152,6 +154,22 @@ std::string GetOtlpDefaultTracesCompression(); std::string GetOtlpDefaultMetricsCompression(); std::string GetOtlpDefaultLogsCompression(); +std::uint32_t GetOtlpDefaultTracesRetryMaxAttempts(); +std::uint32_t GetOtlpDefaultMetricsRetryMaxAttempts(); +std::uint32_t GetOtlpDefaultLogsRetryMaxAttempts(); + +SecondsDecimal GetOtlpDefaultTracesRetryInitialBackoff(); +SecondsDecimal GetOtlpDefaultMetricsRetryInitialBackoff(); +SecondsDecimal GetOtlpDefaultLogsRetryInitialBackoff(); + +SecondsDecimal GetOtlpDefaultTracesRetryMaxBackoff(); +SecondsDecimal GetOtlpDefaultMetricsRetryMaxBackoff(); +SecondsDecimal GetOtlpDefaultLogsRetryMaxBackoff(); + +float GetOtlpDefaultTracesRetryBackoffMultiplier(); +float GetOtlpDefaultMetricsRetryBackoffMultiplier(); +float GetOtlpDefaultLogsRetryBackoffMultiplier(); + } // namespace otlp } // namespace exporter OPENTELEMETRY_END_NAMESPACE diff --git a/exporters/otlp/src/otlp_environment.cc b/exporters/otlp/src/otlp_environment.cc index 276d9fc709..824f1e0010 100644 --- a/exporters/otlp/src/otlp_environment.cc +++ b/exporters/otlp/src/otlp_environment.cc @@ -1125,6 +1125,132 @@ std::string GetOtlpDefaultLogsCompression() return std::string{"none"}; } +std::uint32_t GetUintEnvVarOrDefault(opentelemetry::nostd::string_view signal_env, + opentelemetry::nostd::string_view generic_env, + std::uint32_t default_value) +{ + std::string value; + + if (GetStringDualEnvVar(signal_env.data(), generic_env.data(), value)) + { + return static_cast(std::strtoul(value.c_str(), nullptr, 10)); + } + + return default_value; +} + +float GetFloatEnvVarOrDefault(opentelemetry::nostd::string_view signal_env, + opentelemetry::nostd::string_view generic_env, + float default_value) +{ + std::string value; + + if (GetStringDualEnvVar(signal_env.data(), generic_env.data(), value)) + { + return std::strtof(value.c_str(), nullptr); + } + + return default_value; +} + +SecondsDecimal GetSecondsDecimalEnvVarOrDefault(opentelemetry::nostd::string_view signal_env, + opentelemetry::nostd::string_view generic_env, + SecondsDecimal default_value) +{ + std::string value; + + if (GetStringDualEnvVar(signal_env.data(), generic_env.data(), value)) + { + return SecondsDecimal{std::strtof(value.c_str(), nullptr)}; + } + + return default_value; +} + +std::uint32_t GetOtlpDefaultTracesRetryMaxAttempts() +{ + constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_RETRY_MAX_ATTEMPTS"; + constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"; + return GetUintEnvVarOrDefault(kSignalEnv, kGenericEnv, 5U); +} + +std::uint32_t GetOtlpDefaultMetricsRetryMaxAttempts() +{ + constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_RETRY_MAX_ATTEMPTS"; + constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"; + return GetUintEnvVarOrDefault(kSignalEnv, kGenericEnv, 5U); +} + +std::uint32_t GetOtlpDefaultLogsRetryMaxAttempts() +{ + constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_RETRY_MAX_ATTEMPTS"; + constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"; + return GetUintEnvVarOrDefault(kSignalEnv, kGenericEnv, 5U); +} + +SecondsDecimal GetOtlpDefaultTracesRetryInitialBackoff() +{ + constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_RETRY_INITIAL_BACKOFF"; + constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"; + return GetSecondsDecimalEnvVarOrDefault(kSignalEnv, kGenericEnv, SecondsDecimal{1.0}); +} + +SecondsDecimal GetOtlpDefaultMetricsRetryInitialBackoff() +{ + constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_RETRY_INITIAL_BACKOFF"; + constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"; + return GetSecondsDecimalEnvVarOrDefault(kSignalEnv, kGenericEnv, SecondsDecimal{1.0}); +} + +SecondsDecimal GetOtlpDefaultLogsRetryInitialBackoff() +{ + constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_RETRY_INITIAL_BACKOFF"; + constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"; + return GetSecondsDecimalEnvVarOrDefault(kSignalEnv, kGenericEnv, SecondsDecimal{1.0}); +} + +SecondsDecimal GetOtlpDefaultTracesRetryMaxBackoff() +{ + constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_RETRY_MAX_BACKOFF"; + constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"; + return GetSecondsDecimalEnvVarOrDefault(kSignalEnv, kGenericEnv, SecondsDecimal{5.0}); +} + +SecondsDecimal GetOtlpDefaultMetricsRetryMaxBackoff() +{ + constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_RETRY_MAX_BACKOFF"; + constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"; + return GetSecondsDecimalEnvVarOrDefault(kSignalEnv, kGenericEnv, SecondsDecimal{5.0}); +} + +SecondsDecimal GetOtlpDefaultLogsRetryMaxBackoff() +{ + constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_RETRY_MAX_BACKOFF"; + constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"; + return GetSecondsDecimalEnvVarOrDefault(kSignalEnv, kGenericEnv, SecondsDecimal{5.0}); +} + +float GetOtlpDefaultTracesRetryBackoffMultiplier() +{ + constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_RETRY_BACKOFF_MULTIPLIER"; + constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"; + return GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 1.5f); +} + +float GetOtlpDefaultMetricsRetryBackoffMultiplier() +{ + constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_RETRY_BACKOFF_MULTIPLIER"; + constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"; + return GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 1.5f); +} + +float GetOtlpDefaultLogsRetryBackoffMultiplier() +{ + constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_RETRY_BACKOFF_MULTIPLIER"; + constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"; + return GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 1.5f); +} + } // namespace otlp } // namespace exporter OPENTELEMETRY_END_NAMESPACE From 96fb4476e918527b1d9aed2c37527420c0af0647 Mon Sep 17 00:00:00 2001 From: Alex E Date: Sun, 22 Dec 2024 13:38:33 -0500 Subject: [PATCH 02/16] Update options with retry policy settings --- .../exporters/otlp/otlp_grpc_client_options.h | 12 ++++++++++++ .../otlp/otlp_http_metric_exporter_options.h | 12 ++++++++++++ exporters/otlp/src/otlp_grpc_exporter_options.cc | 5 +++++ .../src/otlp_grpc_log_record_exporter_options.cc | 5 +++++ .../otlp/src/otlp_grpc_metric_exporter_options.cc | 5 +++++ 5 files changed, 39 insertions(+) diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client_options.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client_options.h index 45bd2e8896..b1edbd9273 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client_options.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client_options.h @@ -62,6 +62,18 @@ struct OtlpGrpcClientOptions // Concurrent requests std::size_t max_concurrent_requests; #endif + + /** The maximum number of call attempts, including the original attempt. */ + std::uint32_t retry_policy_max_attempts{}; + + /** The initial backoff delay between retry attempts, random between (0, initial_backoff). */ + SecondsDecimal retry_policy_initial_backoff{}; + + /** The maximum backoff places an upper limit on exponential backoff growth. */ + SecondsDecimal retry_policy_max_backoff{}; + + /** The backoff will be multiplied by this value after each retry attempt. */ + float retry_policy_backoff_multiplier{}; }; } // namespace otlp diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_metric_exporter_options.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_metric_exporter_options.h index 82c91aa51f..99f574c69b 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_metric_exporter_options.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_metric_exporter_options.h @@ -104,6 +104,18 @@ struct OPENTELEMETRY_EXPORT OtlpHttpMetricExporterOptions /** Compression type. */ std::string compression; + + /** The maximum number of call attempts, including the original attempt. */ + std::uint32_t retry_policy_max_attempts{}; + + /** The initial backoff delay between retry attempts, random between (0, initial_backoff). */ + SecondsDecimal retry_policy_initial_backoff{}; + + /** The maximum backoff places an upper limit on exponential backoff growth. */ + SecondsDecimal retry_policy_max_backoff{}; + + /** The backoff will be multiplied by this value after each retry attempt. */ + float retry_policy_backoff_multiplier{}; }; } // namespace otlp diff --git a/exporters/otlp/src/otlp_grpc_exporter_options.cc b/exporters/otlp/src/otlp_grpc_exporter_options.cc index 5fcc77fc2f..12a493111e 100644 --- a/exporters/otlp/src/otlp_grpc_exporter_options.cc +++ b/exporters/otlp/src/otlp_grpc_exporter_options.cc @@ -36,6 +36,11 @@ OtlpGrpcExporterOptions::OtlpGrpcExporterOptions() #ifdef ENABLE_ASYNC_EXPORT max_concurrent_requests = 64; #endif + + retry_policy_max_attempts = GetOtlpDefaultTracesRetryMaxAttempts(); + retry_policy_initial_backoff = GetOtlpDefaultTracesRetryInitialBackoff(); + retry_policy_max_backoff = GetOtlpDefaultTracesRetryMaxBackoff(); + retry_policy_backoff_multiplier = GetOtlpDefaultTracesRetryBackoffMultiplier(); } OtlpGrpcExporterOptions::~OtlpGrpcExporterOptions() {} diff --git a/exporters/otlp/src/otlp_grpc_log_record_exporter_options.cc b/exporters/otlp/src/otlp_grpc_log_record_exporter_options.cc index b98f76a922..c789114ac2 100644 --- a/exporters/otlp/src/otlp_grpc_log_record_exporter_options.cc +++ b/exporters/otlp/src/otlp_grpc_log_record_exporter_options.cc @@ -34,6 +34,11 @@ OtlpGrpcLogRecordExporterOptions::OtlpGrpcLogRecordExporterOptions() #ifdef ENABLE_ASYNC_EXPORT max_concurrent_requests = 64; #endif + + retry_policy_max_attempts = GetOtlpDefaultLogsRetryMaxAttempts(); + retry_policy_initial_backoff = GetOtlpDefaultLogsRetryInitialBackoff(); + retry_policy_max_backoff = GetOtlpDefaultLogsRetryMaxBackoff(); + retry_policy_backoff_multiplier = GetOtlpDefaultLogsRetryBackoffMultiplier(); } OtlpGrpcLogRecordExporterOptions::~OtlpGrpcLogRecordExporterOptions() {} diff --git a/exporters/otlp/src/otlp_grpc_metric_exporter_options.cc b/exporters/otlp/src/otlp_grpc_metric_exporter_options.cc index 0d84ec276f..3d9b1963c6 100644 --- a/exporters/otlp/src/otlp_grpc_metric_exporter_options.cc +++ b/exporters/otlp/src/otlp_grpc_metric_exporter_options.cc @@ -35,6 +35,11 @@ OtlpGrpcMetricExporterOptions::OtlpGrpcMetricExporterOptions() #ifdef ENABLE_ASYNC_EXPORT max_concurrent_requests = 64; #endif + + retry_policy_max_attempts = GetOtlpDefaultMetricsRetryMaxAttempts(); + retry_policy_initial_backoff = GetOtlpDefaultMetricsRetryInitialBackoff(); + retry_policy_max_backoff = GetOtlpDefaultMetricsRetryMaxBackoff(); + retry_policy_backoff_multiplier = GetOtlpDefaultMetricsRetryBackoffMultiplier(); } OtlpGrpcMetricExporterOptions::~OtlpGrpcMetricExporterOptions() {} From e60857581f2f5636e17c068ed417016cf9e3629d Mon Sep 17 00:00:00 2001 From: Alex E Date: Sun, 22 Dec 2024 13:39:10 -0500 Subject: [PATCH 03/16] Fix typo --- exporters/otlp/src/otlp_grpc_exporter.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/otlp/src/otlp_grpc_exporter.cc b/exporters/otlp/src/otlp_grpc_exporter.cc index e4012c2ebf..9dc28a29c4 100644 --- a/exporters/otlp/src/otlp_grpc_exporter.cc +++ b/exporters/otlp/src/otlp_grpc_exporter.cc @@ -105,7 +105,7 @@ sdk::common::ExportResult OtlpGrpcExporter::Export( google::protobuf::ArenaOptions arena_options; // It's easy to allocate datas larger than 1024 when we populate basic resource and attributes arena_options.initial_block_size = 1024; - // When in batch mode, it's easy to export a large number of spans at once, we can alloc a lager + // When in batch mode, it's easy to export a large number of spans at once, we can alloc a larger // block to reduce memory fragments. arena_options.max_block_size = 65536; std::unique_ptr arena{new google::protobuf::Arena{arena_options}}; From 51379007f9387f577a502e1c8bf9843eabfec8f6 Mon Sep 17 00:00:00 2001 From: Alex E Date: Sun, 22 Dec 2024 13:39:56 -0500 Subject: [PATCH 04/16] Use service config in grpc arguments --- exporters/otlp/src/otlp_grpc_client.cc | 42 ++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/exporters/otlp/src/otlp_grpc_client.cc b/exporters/otlp/src/otlp_grpc_client.cc index fe38f16e2b..ec46129e95 100644 --- a/exporters/otlp/src/otlp_grpc_client.cc +++ b/exporters/otlp/src/otlp_grpc_client.cc @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -23,6 +24,7 @@ #include "opentelemetry/common/timestamp.h" #include "opentelemetry/ext/http/common/url_parser.h" #include "opentelemetry/nostd/function_ref.h" +#include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/common/global_log_handler.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -347,6 +349,46 @@ std::shared_ptr OtlpGrpcClient::MakeChannel(const OtlpGrpcClientO grpc_arguments.SetCompressionAlgorithm(GRPC_COMPRESS_GZIP); } + if (options.retry_policy_max_attempts > 0U && + options.retry_policy_initial_backoff > SecondsDecimal::zero() && + options.retry_policy_max_backoff > SecondsDecimal::zero() && + options.retry_policy_backoff_multiplier > 0.f) + { + static const auto kServiceConfigJson = opentelemetry::nostd::string_view{R"( + { + "methodConfig": [ + { + "name": [{}], + "retryPolicy": { + "maxAttempts": %0000000000u, + "initialBackoff": "%.1fs", + "maxBackoff": "%.1fs", + "backoffMultiplier": %.1f, + "retryableStatusCodes": [ + "CANCELLED", + "DEADLINE_EXCEEDED", + "ABORTED", + "OUT_OF_RANGE", + "DATA_LOSS", + "UNAVAILABLE" + ] + } + } + ] + })"}; + + // Allocate string with buffer large enough to hold the formatted json config + auto service_config = std::string(kServiceConfigJson.size(), '\0'); + // Prior to C++17, need to explicitly cast away constness from `data()` buffer + std::snprintf(const_cast(service_config.data()), + service_config.size(), kServiceConfigJson.data(), + options.retry_policy_max_attempts, options.retry_policy_initial_backoff.count(), + options.retry_policy_max_backoff.count(), + options.retry_policy_backoff_multiplier); + + grpc_arguments.SetServiceConfigJSON(service_config); + } + if (options.use_ssl_credentials) { grpc::SslCredentialsOptions ssl_opts; From f06f0b2d3034e90f353194473e4e9b9876e056f0 Mon Sep 17 00:00:00 2001 From: Alex E Date: Sun, 22 Dec 2024 13:40:57 -0500 Subject: [PATCH 05/16] Test config and retry mechanism --- .../otlp/test/otlp_grpc_exporter_test.cc | 199 +++++++++++++++++- .../otlp_grpc_log_record_exporter_test.cc | 52 +++++ .../test/otlp_grpc_metric_exporter_test.cc | 52 +++++ 3 files changed, 302 insertions(+), 1 deletion(-) diff --git a/exporters/otlp/test/otlp_grpc_exporter_test.cc b/exporters/otlp/test/otlp_grpc_exporter_test.cc index b8c651a8fe..7ffdcdffd6 100644 --- a/exporters/otlp/test/otlp_grpc_exporter_test.cc +++ b/exporters/otlp/test/otlp_grpc_exporter_test.cc @@ -17,17 +17,22 @@ // That is because `std::result_of` has been removed in C++20. # include "opentelemetry/exporters/otlp/otlp_grpc_exporter.h" - +# include "opentelemetry/exporters/otlp/otlp_grpc_exporter_factory.h" # include "opentelemetry/exporters/otlp/protobuf_include_prefix.h" +# include "opentelemetry/nostd/shared_ptr.h" +# include "opentelemetry/proto/collector/trace/v1/trace_service.grpc.pb.h" // Problematic code that pulls in Gmock and breaks with vs2019/c++latest : # include "opentelemetry/proto/collector/trace/v1/trace_service_mock.grpc.pb.h" # include "opentelemetry/exporters/otlp/protobuf_include_suffix.h" # include "opentelemetry/sdk/trace/simple_processor.h" +# include "opentelemetry/sdk/trace/simple_processor_factory.h" # include "opentelemetry/sdk/trace/tracer_provider.h" +# include "opentelemetry/sdk/trace/tracer_provider_factory.h" # include "opentelemetry/trace/provider.h" +# include "opentelemetry/trace/tracer_provider.h" # include # include @@ -357,6 +362,198 @@ TEST_F(OtlpGrpcExporterTestPeer, ConfigUnknownInsecureFromEnv) } # endif +# ifndef NO_GETENV +TEST_F(OtlpGrpcExporterTestPeer, ConfigRetryDefaultValues) +{ + std::unique_ptr exporter(new OtlpGrpcExporter()); + const auto options = GetOptions(exporter); + ASSERT_EQ(options.retry_policy_max_attempts, 5); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 1); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 5); + ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 1.5); +} + +TEST_F(OtlpGrpcExporterTestPeer, ConfigRetryValuesFromEnv) +{ + setenv("OTEL_EXPORTER_OTLP_TRACES_RETRY_MAX_ATTEMPTS", "123", 1); + setenv("OTEL_EXPORTER_OTLP_TRACES_RETRY_INITIAL_BACKOFF", "4.5", 1); + setenv("OTEL_EXPORTER_OTLP_TRACES_RETRY_MAX_BACKOFF", "6.7", 1); + setenv("OTEL_EXPORTER_OTLP_TRACES_RETRY_BACKOFF_MULTIPLIER", "8.9", 1); + + std::unique_ptr exporter(new OtlpGrpcExporter()); + const auto options = GetOptions(exporter); + ASSERT_EQ(options.retry_policy_max_attempts, 123); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 4.5); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 6.7); + ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 8.9); + + unsetenv("OTEL_EXPORTER_OTLP_TRACES_RETRY_MAX_ATTEMPTS"); + unsetenv("OTEL_EXPORTER_OTLP_TRACES_RETRY_INITIAL_BACKOFF"); + unsetenv("OTEL_EXPORTER_OTLP_TRACES_RETRY_MAX_BACKOFF"); + unsetenv("OTEL_EXPORTER_OTLP_TRACES_RETRY_BACKOFF_MULTIPLIER"); +} + +TEST_F(OtlpGrpcExporterTestPeer, ConfigRetryGenericValuesFromEnv) +{ + setenv("OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS", "321", 1); + setenv("OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF", "5.4", 1); + setenv("OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF", "7.6", 1); + setenv("OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER", "9.8", 1); + + std::unique_ptr exporter(new OtlpGrpcExporter()); + const auto options = GetOptions(exporter); + ASSERT_EQ(options.retry_policy_max_attempts, 321); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 5.4); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 7.6); + ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 9.8); + + unsetenv("OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"); + unsetenv("OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"); + unsetenv("OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"); + unsetenv("OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"); +} +# endif // NO_GETENV + +struct TestTraceService : public opentelemetry::proto::collector::trace::v1::TraceService::Service +{ + TestTraceService(std::vector status_codes) : status_codes_(status_codes) {} + + inline grpc::Status Export( + grpc::ServerContext *context, + const opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest *request, + opentelemetry::proto::collector::trace::v1::ExportTraceServiceResponse *response) override + { + ++request_count_; + return grpc::Status(status_codes_.at(index_++ % status_codes_.size()), "TEST!"); + } + + size_t request_count_ = 0UL; + size_t index_ = 0UL; + std::vector status_codes_; +}; + +class OtlpGrpcExporterRetryIntegrationTests + : public ::testing::TestWithParam, std::size_t>> +{}; + +INSTANTIATE_TEST_SUITE_P( + StatusCodes, + OtlpGrpcExporterRetryIntegrationTests, + testing::Values( + // With retry policy enabled + std::make_tuple(true, std::vector{grpc::StatusCode::CANCELLED}, 5), + std::make_tuple(true, std::vector{grpc::StatusCode::UNKNOWN}, 1), + std::make_tuple(true, std::vector{grpc::StatusCode::INVALID_ARGUMENT}, 1), + std::make_tuple(true, std::vector{grpc::StatusCode::DEADLINE_EXCEEDED}, 5), + std::make_tuple(true, std::vector{grpc::StatusCode::NOT_FOUND}, 1), + std::make_tuple(true, std::vector{grpc::StatusCode::ALREADY_EXISTS}, 1), + std::make_tuple(true, std::vector{grpc::StatusCode::PERMISSION_DENIED}, 1), + std::make_tuple(true, std::vector{grpc::StatusCode::UNAUTHENTICATED}, 1), + std::make_tuple(true, std::vector{grpc::StatusCode::RESOURCE_EXHAUSTED}, 1), + std::make_tuple(true, std::vector{grpc::StatusCode::FAILED_PRECONDITION}, 1), + std::make_tuple(true, std::vector{grpc::StatusCode::ABORTED}, 5), + std::make_tuple(true, std::vector{grpc::StatusCode::OUT_OF_RANGE}, 5), + std::make_tuple(true, std::vector{grpc::StatusCode::UNIMPLEMENTED}, 1), + std::make_tuple(true, std::vector{grpc::StatusCode::INTERNAL}, 1), + std::make_tuple(true, std::vector{grpc::StatusCode::UNAVAILABLE}, 5), + std::make_tuple(true, std::vector{grpc::StatusCode::DATA_LOSS}, 5), + std::make_tuple(true, std::vector{grpc::StatusCode::OK}, 1), + std::make_tuple(true, + std::vector{grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::ABORTED, + grpc::StatusCode::OUT_OF_RANGE, grpc::StatusCode::DATA_LOSS}, + 5), + std::make_tuple(true, + std::vector{grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::UNAVAILABLE, + grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::OK}, + 4), + std::make_tuple(true, + std::vector{grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::CANCELLED, + grpc::StatusCode::DEADLINE_EXCEEDED, grpc::StatusCode::OK}, + 4), + // With retry policy disabled + std::make_tuple(false, std::vector{grpc::StatusCode::CANCELLED}, 1), + std::make_tuple(false, std::vector{grpc::StatusCode::UNKNOWN}, 1), + std::make_tuple(false, std::vector{grpc::StatusCode::INVALID_ARGUMENT}, 1), + std::make_tuple(false, std::vector{grpc::StatusCode::DEADLINE_EXCEEDED}, 1), + std::make_tuple(false, std::vector{grpc::StatusCode::NOT_FOUND}, 1), + std::make_tuple(false, std::vector{grpc::StatusCode::ALREADY_EXISTS}, 1), + std::make_tuple(false, std::vector{grpc::StatusCode::PERMISSION_DENIED}, 1), + std::make_tuple(false, std::vector{grpc::StatusCode::UNAUTHENTICATED}, 1), + std::make_tuple(false, std::vector{grpc::StatusCode::RESOURCE_EXHAUSTED}, 1), + std::make_tuple(false, std::vector{grpc::StatusCode::FAILED_PRECONDITION}, 1), + std::make_tuple(false, std::vector{grpc::StatusCode::ABORTED}, 1), + std::make_tuple(false, std::vector{grpc::StatusCode::OUT_OF_RANGE}, 1), + std::make_tuple(false, std::vector{grpc::StatusCode::UNIMPLEMENTED}, 1), + std::make_tuple(false, std::vector{grpc::StatusCode::INTERNAL}, 1), + std::make_tuple(false, std::vector{grpc::StatusCode::UNAVAILABLE}, 1), + std::make_tuple(false, std::vector{grpc::StatusCode::DATA_LOSS}, 1), + std::make_tuple(false, std::vector{grpc::StatusCode::OK}, 1), + std::make_tuple(false, + std::vector{grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::ABORTED, + grpc::StatusCode::OUT_OF_RANGE, grpc::StatusCode::DATA_LOSS}, + 1), + std::make_tuple(false, + std::vector{grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::UNAVAILABLE, + grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::OK}, + 1), + std::make_tuple(false, + std::vector{grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::CANCELLED, + grpc::StatusCode::DEADLINE_EXCEEDED, grpc::StatusCode::OK}, + 1))); + +TEST_P(OtlpGrpcExporterRetryIntegrationTests, StatusCodes) +{ + namespace otlp = opentelemetry::exporter::otlp; + namespace trace_sdk = opentelemetry::sdk::trace; + + const auto is_retry_enabled = std::get<0>(GetParam()); + const auto status_codes = std::get<1>(GetParam()); + const auto expected_attempts = std::get<2>(GetParam()); + TestTraceService service(status_codes); + std::unique_ptr server; + + std::thread server_thread([&server, &service]() { + std::string address("0.0.0.0:4317"); + grpc::ServerBuilder builder; + builder.RegisterService(&service); + builder.AddListeningPort(address, grpc::InsecureServerCredentials()); + server = builder.BuildAndStart(); + server->Wait(); + }); + + otlp::OtlpGrpcExporterOptions opts{}; + + if (is_retry_enabled) + { + opts.retry_policy_max_attempts = 5; + opts.retry_policy_initial_backoff = SecondsDecimal{0.1}; + opts.retry_policy_max_backoff = SecondsDecimal{5}; + opts.retry_policy_backoff_multiplier = 1; + } + else + { + opts.retry_policy_max_attempts = 0; + opts.retry_policy_initial_backoff = SecondsDecimal{0}; + opts.retry_policy_max_backoff = SecondsDecimal{0}; + opts.retry_policy_backoff_multiplier = 0; + } + + auto exporter = otlp::OtlpGrpcExporterFactory::Create(opts); + auto processor = trace_sdk::SimpleSpanProcessorFactory::Create(std::move(exporter)); + auto provider = trace_sdk::TracerProviderFactory::Create(std::move(processor)); + provider->GetTracer("Test tracer")->StartSpan("Test span")->End(); + + ASSERT_TRUE(server); + server->Shutdown(); + + if (server_thread.joinable()) + { + server_thread.join(); + } + + ASSERT_EQ(expected_attempts, service.request_count_); +} + } // namespace otlp } // namespace exporter OPENTELEMETRY_END_NAMESPACE diff --git a/exporters/otlp/test/otlp_grpc_log_record_exporter_test.cc b/exporters/otlp/test/otlp_grpc_log_record_exporter_test.cc index a2604de096..7f8bbada82 100644 --- a/exporters/otlp/test/otlp_grpc_log_record_exporter_test.cc +++ b/exporters/otlp/test/otlp_grpc_log_record_exporter_test.cc @@ -466,6 +466,58 @@ TEST_F(OtlpGrpcLogRecordExporterTestPeer, ShareClientTest) trace_provider = opentelemetry::nostd::shared_ptr(); } +#ifndef NO_GETENV +TEST_F(OtlpGrpcLogRecordExporterTestPeer, ConfigRetryDefaultValues) +{ + std::unique_ptr exporter(new OtlpGrpcLogRecordExporter()); + const auto options = GetOptions(exporter); + ASSERT_EQ(options.retry_policy_max_attempts, 5); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 1); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 5); + ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 1.5); +} + +TEST_F(OtlpGrpcLogRecordExporterTestPeer, ConfigRetryValuesFromEnv) +{ + setenv("OTEL_EXPORTER_OTLP_LOGS_RETRY_MAX_ATTEMPTS", "123", 1); + setenv("OTEL_EXPORTER_OTLP_LOGS_RETRY_INITIAL_BACKOFF", "4.5", 1); + setenv("OTEL_EXPORTER_OTLP_LOGS_RETRY_MAX_BACKOFF", "6.7", 1); + setenv("OTEL_EXPORTER_OTLP_LOGS_RETRY_BACKOFF_MULTIPLIER", "8.9", 1); + + std::unique_ptr exporter(new OtlpGrpcLogRecordExporter()); + const auto options = GetOptions(exporter); + ASSERT_EQ(options.retry_policy_max_attempts, 123); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 4.5); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 6.7); + ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 8.9); + + unsetenv("OTEL_EXPORTER_OTLP_LOGS_RETRY_MAX_ATTEMPTS"); + unsetenv("OTEL_EXPORTER_OTLP_LOGS_RETRY_INITIAL_BACKOFF"); + unsetenv("OTEL_EXPORTER_OTLP_LOGS_RETRY_MAX_BACKOFF"); + unsetenv("OTEL_EXPORTER_OTLP_LOGS_RETRY_BACKOFF_MULTIPLIER"); +} + +TEST_F(OtlpGrpcLogRecordExporterTestPeer, ConfigRetryGenericValuesFromEnv) +{ + setenv("OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS", "321", 1); + setenv("OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF", "5.4", 1); + setenv("OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF", "7.6", 1); + setenv("OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER", "9.8", 1); + + std::unique_ptr exporter(new OtlpGrpcLogRecordExporter()); + const auto options = GetOptions(exporter); + ASSERT_EQ(options.retry_policy_max_attempts, 321); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 5.4); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 7.6); + ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 9.8); + + unsetenv("OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"); + unsetenv("OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"); + unsetenv("OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"); + unsetenv("OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"); +} +#endif // NO_GETENV + } // namespace otlp } // namespace exporter OPENTELEMETRY_END_NAMESPACE diff --git a/exporters/otlp/test/otlp_grpc_metric_exporter_test.cc b/exporters/otlp/test/otlp_grpc_metric_exporter_test.cc index 6689233a9d..e47f048289 100644 --- a/exporters/otlp/test/otlp_grpc_metric_exporter_test.cc +++ b/exporters/otlp/test/otlp_grpc_metric_exporter_test.cc @@ -204,6 +204,58 @@ TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigUnknownInsecureFromEnv) } # endif +# ifndef NO_GETENV +TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigRetryDefaultValues) +{ + std::unique_ptr exporter(new OtlpGrpcMetricExporter()); + const auto options = GetOptions(exporter); + ASSERT_EQ(options.retry_policy_max_attempts, 5); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 1); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 5); + ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 1.5); +} + +TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigRetryValuesFromEnv) +{ + setenv("OTEL_EXPORTER_OTLP_METRICS_RETRY_MAX_ATTEMPTS", "123", 1); + setenv("OTEL_EXPORTER_OTLP_METRICS_RETRY_INITIAL_BACKOFF", "4.5", 1); + setenv("OTEL_EXPORTER_OTLP_METRICS_RETRY_MAX_BACKOFF", "6.7", 1); + setenv("OTEL_EXPORTER_OTLP_METRICS_RETRY_BACKOFF_MULTIPLIER", "8.9", 1); + + std::unique_ptr exporter(new OtlpGrpcMetricExporter()); + const auto options = GetOptions(exporter); + ASSERT_EQ(options.retry_policy_max_attempts, 123); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 4.5); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 6.7); + ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 8.9); + + unsetenv("OTEL_EXPORTER_OTLP_METRICS_RETRY_MAX_ATTEMPTS"); + unsetenv("OTEL_EXPORTER_OTLP_METRICS_RETRY_INITIAL_BACKOFF"); + unsetenv("OTEL_EXPORTER_OTLP_METRICS_RETRY_MAX_BACKOFF"); + unsetenv("OTEL_EXPORTER_OTLP_METRICS_RETRY_BACKOFF_MULTIPLIER"); +} + +TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigRetryGenericValuesFromEnv) +{ + setenv("OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS", "321", 1); + setenv("OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF", "5.4", 1); + setenv("OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF", "7.6", 1); + setenv("OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER", "9.8", 1); + + std::unique_ptr exporter(new OtlpGrpcMetricExporter()); + const auto options = GetOptions(exporter); + ASSERT_EQ(options.retry_policy_max_attempts, 321); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 5.4); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 7.6); + ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 9.8); + + unsetenv("OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"); + unsetenv("OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"); + unsetenv("OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"); + unsetenv("OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"); +} +# endif // NO_GETENV + } // namespace otlp } // namespace exporter OPENTELEMETRY_END_NAMESPACE From 863e0ed19ded866245292de1412c0b89108c4ada Mon Sep 17 00:00:00 2001 From: Alex E Date: Sun, 22 Dec 2024 16:03:24 -0500 Subject: [PATCH 06/16] Reduce code bloat a bit --- exporters/otlp/src/otlp_environment.cc | 82 +++++++++++--------------- 1 file changed, 34 insertions(+), 48 deletions(-) diff --git a/exporters/otlp/src/otlp_environment.cc b/exporters/otlp/src/otlp_environment.cc index 824f1e0010..a9e43acb08 100644 --- a/exporters/otlp/src/otlp_environment.cc +++ b/exporters/otlp/src/otlp_environment.cc @@ -80,6 +80,34 @@ static bool GetStringDualEnvVar(const char *signal_name, return exists; } +static std::uint32_t GetUintEnvVarOrDefault(opentelemetry::nostd::string_view signal_env, + opentelemetry::nostd::string_view generic_env, + std::uint32_t default_value) +{ + std::string value; + + if (GetStringDualEnvVar(signal_env.data(), generic_env.data(), value)) + { + return static_cast(std::strtoul(value.c_str(), nullptr, 10)); + } + + return default_value; +} + +static float GetFloatEnvVarOrDefault(opentelemetry::nostd::string_view signal_env, + opentelemetry::nostd::string_view generic_env, + float default_value) +{ + std::string value; + + if (GetStringDualEnvVar(signal_env.data(), generic_env.data(), value)) + { + return std::strtof(value.c_str(), nullptr); + } + + return default_value; +} + std::string GetOtlpDefaultGrpcTracesEndpoint() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT"; @@ -1125,48 +1153,6 @@ std::string GetOtlpDefaultLogsCompression() return std::string{"none"}; } -std::uint32_t GetUintEnvVarOrDefault(opentelemetry::nostd::string_view signal_env, - opentelemetry::nostd::string_view generic_env, - std::uint32_t default_value) -{ - std::string value; - - if (GetStringDualEnvVar(signal_env.data(), generic_env.data(), value)) - { - return static_cast(std::strtoul(value.c_str(), nullptr, 10)); - } - - return default_value; -} - -float GetFloatEnvVarOrDefault(opentelemetry::nostd::string_view signal_env, - opentelemetry::nostd::string_view generic_env, - float default_value) -{ - std::string value; - - if (GetStringDualEnvVar(signal_env.data(), generic_env.data(), value)) - { - return std::strtof(value.c_str(), nullptr); - } - - return default_value; -} - -SecondsDecimal GetSecondsDecimalEnvVarOrDefault(opentelemetry::nostd::string_view signal_env, - opentelemetry::nostd::string_view generic_env, - SecondsDecimal default_value) -{ - std::string value; - - if (GetStringDualEnvVar(signal_env.data(), generic_env.data(), value)) - { - return SecondsDecimal{std::strtof(value.c_str(), nullptr)}; - } - - return default_value; -} - std::uint32_t GetOtlpDefaultTracesRetryMaxAttempts() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_RETRY_MAX_ATTEMPTS"; @@ -1192,42 +1178,42 @@ SecondsDecimal GetOtlpDefaultTracesRetryInitialBackoff() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_RETRY_INITIAL_BACKOFF"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"; - return GetSecondsDecimalEnvVarOrDefault(kSignalEnv, kGenericEnv, SecondsDecimal{1.0}); + return SecondsDecimal{GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 1.0)}; } SecondsDecimal GetOtlpDefaultMetricsRetryInitialBackoff() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_RETRY_INITIAL_BACKOFF"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"; - return GetSecondsDecimalEnvVarOrDefault(kSignalEnv, kGenericEnv, SecondsDecimal{1.0}); + return SecondsDecimal{GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 1.0)}; } SecondsDecimal GetOtlpDefaultLogsRetryInitialBackoff() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_RETRY_INITIAL_BACKOFF"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"; - return GetSecondsDecimalEnvVarOrDefault(kSignalEnv, kGenericEnv, SecondsDecimal{1.0}); + return SecondsDecimal{GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 1.0)}; } SecondsDecimal GetOtlpDefaultTracesRetryMaxBackoff() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_RETRY_MAX_BACKOFF"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"; - return GetSecondsDecimalEnvVarOrDefault(kSignalEnv, kGenericEnv, SecondsDecimal{5.0}); + return SecondsDecimal{GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 5.0)}; } SecondsDecimal GetOtlpDefaultMetricsRetryMaxBackoff() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_RETRY_MAX_BACKOFF"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"; - return GetSecondsDecimalEnvVarOrDefault(kSignalEnv, kGenericEnv, SecondsDecimal{5.0}); + return SecondsDecimal{GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 5.0)}; } SecondsDecimal GetOtlpDefaultLogsRetryMaxBackoff() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_RETRY_MAX_BACKOFF"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"; - return GetSecondsDecimalEnvVarOrDefault(kSignalEnv, kGenericEnv, SecondsDecimal{5.0}); + return SecondsDecimal{GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 5.0)}; } float GetOtlpDefaultTracesRetryBackoffMultiplier() From 651c1da9444ddb4d10457b14b974932b0f592ddc Mon Sep 17 00:00:00 2001 From: Alex E Date: Sun, 22 Dec 2024 16:42:07 -0500 Subject: [PATCH 07/16] Fixes for CI tests --- exporters/otlp/CMakeLists.txt | 2 +- .../otlp/otlp_http_metric_exporter_options.h | 12 -- .../otlp/test/otlp_grpc_exporter_test.cc | 106 ++++++++++-------- 3 files changed, 58 insertions(+), 62 deletions(-) diff --git a/exporters/otlp/CMakeLists.txt b/exporters/otlp/CMakeLists.txt index cd4e1b62fa..bf0ae1f35b 100644 --- a/exporters/otlp/CMakeLists.txt +++ b/exporters/otlp/CMakeLists.txt @@ -317,7 +317,7 @@ if(BUILD_TESTING) add_executable(otlp_grpc_exporter_test test/otlp_grpc_exporter_test.cc) target_link_libraries( otlp_grpc_exporter_test ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} - ${GMOCK_LIB} opentelemetry_exporter_otlp_grpc) + ${GMOCK_LIB} opentelemetry_exporter_otlp_grpc gRPC::grpc++) gtest_add_tests( TARGET otlp_grpc_exporter_test TEST_PREFIX exporter.otlp. diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_metric_exporter_options.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_metric_exporter_options.h index 99f574c69b..82c91aa51f 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_metric_exporter_options.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_metric_exporter_options.h @@ -104,18 +104,6 @@ struct OPENTELEMETRY_EXPORT OtlpHttpMetricExporterOptions /** Compression type. */ std::string compression; - - /** The maximum number of call attempts, including the original attempt. */ - std::uint32_t retry_policy_max_attempts{}; - - /** The initial backoff delay between retry attempts, random between (0, initial_backoff). */ - SecondsDecimal retry_policy_initial_backoff{}; - - /** The maximum backoff places an upper limit on exponential backoff growth. */ - SecondsDecimal retry_policy_max_backoff{}; - - /** The backoff will be multiplied by this value after each retry attempt. */ - float retry_policy_backoff_multiplier{}; }; } // namespace otlp diff --git a/exporters/otlp/test/otlp_grpc_exporter_test.cc b/exporters/otlp/test/otlp_grpc_exporter_test.cc index 7ffdcdffd6..5a0e7d7a90 100644 --- a/exporters/otlp/test/otlp_grpc_exporter_test.cc +++ b/exporters/otlp/test/otlp_grpc_exporter_test.cc @@ -21,10 +21,11 @@ # include "opentelemetry/exporters/otlp/protobuf_include_prefix.h" # include "opentelemetry/nostd/shared_ptr.h" -# include "opentelemetry/proto/collector/trace/v1/trace_service.grpc.pb.h" // Problematic code that pulls in Gmock and breaks with vs2019/c++latest : # include "opentelemetry/proto/collector/trace/v1/trace_service_mock.grpc.pb.h" +# include "opentelemetry/proto/collector/trace/v1/trace_service.grpc.pb.h" + # include "opentelemetry/exporters/otlp/protobuf_include_suffix.h" # include "opentelemetry/sdk/trace/simple_processor.h" @@ -432,8 +433,10 @@ struct TestTraceService : public opentelemetry::proto::collector::trace::v1::Tra std::vector status_codes_; }; +using StatusCodeVector = std::vector; + class OtlpGrpcExporterRetryIntegrationTests - : public ::testing::TestWithParam, std::size_t>> + : public ::testing::TestWithParam> {}; INSTANTIATE_TEST_SUITE_P( @@ -441,64 +444,68 @@ INSTANTIATE_TEST_SUITE_P( OtlpGrpcExporterRetryIntegrationTests, testing::Values( // With retry policy enabled - std::make_tuple(true, std::vector{grpc::StatusCode::CANCELLED}, 5), - std::make_tuple(true, std::vector{grpc::StatusCode::UNKNOWN}, 1), - std::make_tuple(true, std::vector{grpc::StatusCode::INVALID_ARGUMENT}, 1), - std::make_tuple(true, std::vector{grpc::StatusCode::DEADLINE_EXCEEDED}, 5), - std::make_tuple(true, std::vector{grpc::StatusCode::NOT_FOUND}, 1), - std::make_tuple(true, std::vector{grpc::StatusCode::ALREADY_EXISTS}, 1), - std::make_tuple(true, std::vector{grpc::StatusCode::PERMISSION_DENIED}, 1), - std::make_tuple(true, std::vector{grpc::StatusCode::UNAUTHENTICATED}, 1), - std::make_tuple(true, std::vector{grpc::StatusCode::RESOURCE_EXHAUSTED}, 1), - std::make_tuple(true, std::vector{grpc::StatusCode::FAILED_PRECONDITION}, 1), - std::make_tuple(true, std::vector{grpc::StatusCode::ABORTED}, 5), - std::make_tuple(true, std::vector{grpc::StatusCode::OUT_OF_RANGE}, 5), - std::make_tuple(true, std::vector{grpc::StatusCode::UNIMPLEMENTED}, 1), - std::make_tuple(true, std::vector{grpc::StatusCode::INTERNAL}, 1), - std::make_tuple(true, std::vector{grpc::StatusCode::UNAVAILABLE}, 5), - std::make_tuple(true, std::vector{grpc::StatusCode::DATA_LOSS}, 5), - std::make_tuple(true, std::vector{grpc::StatusCode::OK}, 1), + std::make_tuple(true, StatusCodeVector{grpc::StatusCode::CANCELLED}, 5), + std::make_tuple(true, StatusCodeVector{grpc::StatusCode::UNKNOWN}, 1), + std::make_tuple(true, StatusCodeVector{grpc::StatusCode::INVALID_ARGUMENT}, 1), + std::make_tuple(true, StatusCodeVector{grpc::StatusCode::DEADLINE_EXCEEDED}, 5), + std::make_tuple(true, StatusCodeVector{grpc::StatusCode::NOT_FOUND}, 1), + std::make_tuple(true, StatusCodeVector{grpc::StatusCode::ALREADY_EXISTS}, 1), + std::make_tuple(true, StatusCodeVector{grpc::StatusCode::PERMISSION_DENIED}, 1), + std::make_tuple(true, StatusCodeVector{grpc::StatusCode::UNAUTHENTICATED}, 1), + std::make_tuple(true, StatusCodeVector{grpc::StatusCode::RESOURCE_EXHAUSTED}, 1), + std::make_tuple(true, StatusCodeVector{grpc::StatusCode::FAILED_PRECONDITION}, 1), + std::make_tuple(true, StatusCodeVector{grpc::StatusCode::ABORTED}, 5), + std::make_tuple(true, StatusCodeVector{grpc::StatusCode::OUT_OF_RANGE}, 5), + std::make_tuple(true, StatusCodeVector{grpc::StatusCode::UNIMPLEMENTED}, 1), + std::make_tuple(true, StatusCodeVector{grpc::StatusCode::INTERNAL}, 1), + std::make_tuple(true, StatusCodeVector{grpc::StatusCode::UNAVAILABLE}, 5), + std::make_tuple(true, StatusCodeVector{grpc::StatusCode::DATA_LOSS}, 5), + std::make_tuple(true, StatusCodeVector{grpc::StatusCode::OK}, 1), std::make_tuple(true, - std::vector{grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::ABORTED, - grpc::StatusCode::OUT_OF_RANGE, grpc::StatusCode::DATA_LOSS}, + StatusCodeVector{grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::ABORTED, + grpc::StatusCode::OUT_OF_RANGE, + grpc::StatusCode::DATA_LOSS}, 5), std::make_tuple(true, - std::vector{grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::UNAVAILABLE, - grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::OK}, + StatusCodeVector{grpc::StatusCode::UNAVAILABLE, + grpc::StatusCode::UNAVAILABLE, + grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::OK}, 4), std::make_tuple(true, - std::vector{grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::CANCELLED, - grpc::StatusCode::DEADLINE_EXCEEDED, grpc::StatusCode::OK}, + StatusCodeVector{grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::CANCELLED, + grpc::StatusCode::DEADLINE_EXCEEDED, grpc::StatusCode::OK}, 4), // With retry policy disabled - std::make_tuple(false, std::vector{grpc::StatusCode::CANCELLED}, 1), - std::make_tuple(false, std::vector{grpc::StatusCode::UNKNOWN}, 1), - std::make_tuple(false, std::vector{grpc::StatusCode::INVALID_ARGUMENT}, 1), - std::make_tuple(false, std::vector{grpc::StatusCode::DEADLINE_EXCEEDED}, 1), - std::make_tuple(false, std::vector{grpc::StatusCode::NOT_FOUND}, 1), - std::make_tuple(false, std::vector{grpc::StatusCode::ALREADY_EXISTS}, 1), - std::make_tuple(false, std::vector{grpc::StatusCode::PERMISSION_DENIED}, 1), - std::make_tuple(false, std::vector{grpc::StatusCode::UNAUTHENTICATED}, 1), - std::make_tuple(false, std::vector{grpc::StatusCode::RESOURCE_EXHAUSTED}, 1), - std::make_tuple(false, std::vector{grpc::StatusCode::FAILED_PRECONDITION}, 1), - std::make_tuple(false, std::vector{grpc::StatusCode::ABORTED}, 1), - std::make_tuple(false, std::vector{grpc::StatusCode::OUT_OF_RANGE}, 1), - std::make_tuple(false, std::vector{grpc::StatusCode::UNIMPLEMENTED}, 1), - std::make_tuple(false, std::vector{grpc::StatusCode::INTERNAL}, 1), - std::make_tuple(false, std::vector{grpc::StatusCode::UNAVAILABLE}, 1), - std::make_tuple(false, std::vector{grpc::StatusCode::DATA_LOSS}, 1), - std::make_tuple(false, std::vector{grpc::StatusCode::OK}, 1), + std::make_tuple(false, StatusCodeVector{grpc::StatusCode::CANCELLED}, 1), + std::make_tuple(false, StatusCodeVector{grpc::StatusCode::UNKNOWN}, 1), + std::make_tuple(false, StatusCodeVector{grpc::StatusCode::INVALID_ARGUMENT}, 1), + std::make_tuple(false, StatusCodeVector{grpc::StatusCode::DEADLINE_EXCEEDED}, 1), + std::make_tuple(false, StatusCodeVector{grpc::StatusCode::NOT_FOUND}, 1), + std::make_tuple(false, StatusCodeVector{grpc::StatusCode::ALREADY_EXISTS}, 1), + std::make_tuple(false, StatusCodeVector{grpc::StatusCode::PERMISSION_DENIED}, 1), + std::make_tuple(false, StatusCodeVector{grpc::StatusCode::UNAUTHENTICATED}, 1), + std::make_tuple(false, StatusCodeVector{grpc::StatusCode::RESOURCE_EXHAUSTED}, 1), + std::make_tuple(false, StatusCodeVector{grpc::StatusCode::FAILED_PRECONDITION}, 1), + std::make_tuple(false, StatusCodeVector{grpc::StatusCode::ABORTED}, 1), + std::make_tuple(false, StatusCodeVector{grpc::StatusCode::OUT_OF_RANGE}, 1), + std::make_tuple(false, StatusCodeVector{grpc::StatusCode::UNIMPLEMENTED}, 1), + std::make_tuple(false, StatusCodeVector{grpc::StatusCode::INTERNAL}, 1), + std::make_tuple(false, StatusCodeVector{grpc::StatusCode::UNAVAILABLE}, 1), + std::make_tuple(false, StatusCodeVector{grpc::StatusCode::DATA_LOSS}, 1), + std::make_tuple(false, StatusCodeVector{grpc::StatusCode::OK}, 1), std::make_tuple(false, - std::vector{grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::ABORTED, - grpc::StatusCode::OUT_OF_RANGE, grpc::StatusCode::DATA_LOSS}, + StatusCodeVector{grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::ABORTED, + grpc::StatusCode::OUT_OF_RANGE, + grpc::StatusCode::DATA_LOSS}, 1), std::make_tuple(false, - std::vector{grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::UNAVAILABLE, - grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::OK}, + StatusCodeVector{grpc::StatusCode::UNAVAILABLE, + grpc::StatusCode::UNAVAILABLE, + grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::OK}, 1), std::make_tuple(false, - std::vector{grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::CANCELLED, - grpc::StatusCode::DEADLINE_EXCEEDED, grpc::StatusCode::OK}, + StatusCodeVector{grpc::StatusCode::UNAVAILABLE, grpc::StatusCode::CANCELLED, + grpc::StatusCode::DEADLINE_EXCEEDED, grpc::StatusCode::OK}, 1))); TEST_P(OtlpGrpcExporterRetryIntegrationTests, StatusCodes) @@ -513,7 +520,7 @@ TEST_P(OtlpGrpcExporterRetryIntegrationTests, StatusCodes) std::unique_ptr server; std::thread server_thread([&server, &service]() { - std::string address("0.0.0.0:4317"); + std::string address("localhost:4317"); grpc::ServerBuilder builder; builder.RegisterService(&service); builder.AddListeningPort(address, grpc::InsecureServerCredentials()); @@ -542,6 +549,7 @@ TEST_P(OtlpGrpcExporterRetryIntegrationTests, StatusCodes) auto processor = trace_sdk::SimpleSpanProcessorFactory::Create(std::move(exporter)); auto provider = trace_sdk::TracerProviderFactory::Create(std::move(processor)); provider->GetTracer("Test tracer")->StartSpan("Test span")->End(); + provider->ForceFlush(); ASSERT_TRUE(server); server->Shutdown(); From 7d422cd9b15f22d93e3dea8949572f70ce85cbf0 Mon Sep 17 00:00:00 2001 From: Alex E Date: Mon, 23 Dec 2024 17:58:41 -0500 Subject: [PATCH 08/16] Remove redundant guards --- exporters/otlp/test/otlp_grpc_exporter_test.cc | 10 ---------- exporters/otlp/test/otlp_grpc_metric_exporter_test.cc | 10 ---------- 2 files changed, 20 deletions(-) diff --git a/exporters/otlp/test/otlp_grpc_exporter_test.cc b/exporters/otlp/test/otlp_grpc_exporter_test.cc index 5a0e7d7a90..3588219c77 100644 --- a/exporters/otlp/test/otlp_grpc_exporter_test.cc +++ b/exporters/otlp/test/otlp_grpc_exporter_test.cc @@ -291,9 +291,7 @@ TEST_F(OtlpGrpcExporterTestPeer, ConfigFromEnv) unsetenv("OTEL_EXPORTER_OTLP_HEADERS"); unsetenv("OTEL_EXPORTER_OTLP_TRACES_HEADERS"); } -# endif -# ifndef NO_GETENV // Test exporter configuration options with use_ssl_credentials TEST_F(OtlpGrpcExporterTestPeer, ConfigHttpsSecureFromEnv) { @@ -309,9 +307,7 @@ TEST_F(OtlpGrpcExporterTestPeer, ConfigHttpsSecureFromEnv) unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT"); unsetenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE"); } -# endif -# ifndef NO_GETENV // Test exporter configuration options with use_ssl_credentials TEST_F(OtlpGrpcExporterTestPeer, ConfigHttpInsecureFromEnv) { @@ -327,9 +323,7 @@ TEST_F(OtlpGrpcExporterTestPeer, ConfigHttpInsecureFromEnv) unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT"); unsetenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE"); } -# endif -# ifndef NO_GETENV // Test exporter configuration options with use_ssl_credentials TEST_F(OtlpGrpcExporterTestPeer, ConfigUnknownSecureFromEnv) { @@ -344,9 +338,7 @@ TEST_F(OtlpGrpcExporterTestPeer, ConfigUnknownSecureFromEnv) unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT"); unsetenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE"); } -# endif -# ifndef NO_GETENV // Test exporter configuration options with use_ssl_credentials TEST_F(OtlpGrpcExporterTestPeer, ConfigUnknownInsecureFromEnv) { @@ -361,9 +353,7 @@ TEST_F(OtlpGrpcExporterTestPeer, ConfigUnknownInsecureFromEnv) unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT"); unsetenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE"); } -# endif -# ifndef NO_GETENV TEST_F(OtlpGrpcExporterTestPeer, ConfigRetryDefaultValues) { std::unique_ptr exporter(new OtlpGrpcExporter()); diff --git a/exporters/otlp/test/otlp_grpc_metric_exporter_test.cc b/exporters/otlp/test/otlp_grpc_metric_exporter_test.cc index e47f048289..d0a73c1d14 100644 --- a/exporters/otlp/test/otlp_grpc_metric_exporter_test.cc +++ b/exporters/otlp/test/otlp_grpc_metric_exporter_test.cc @@ -132,9 +132,7 @@ TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigFromEnv) unsetenv("OTEL_EXPORTER_OTLP_HEADERS"); unsetenv("OTEL_EXPORTER_OTLP_METRICS_HEADERS"); } -# endif -# ifndef NO_GETENV // Test exporter configuration options with use_ssl_credentials TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigHttpsSecureFromEnv) { @@ -150,9 +148,7 @@ TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigHttpsSecureFromEnv) unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT"); unsetenv("OTEL_EXPORTER_OTLP_METRICS_INSECURE"); } -# endif -# ifndef NO_GETENV // Test exporter configuration options with use_ssl_credentials TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigHttpInsecureFromEnv) { @@ -168,9 +164,7 @@ TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigHttpInsecureFromEnv) unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT"); unsetenv("OTEL_EXPORTER_OTLP_METRICS_INSECURE"); } -# endif -# ifndef NO_GETENV // Test exporter configuration options with use_ssl_credentials TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigUnknownSecureFromEnv) { @@ -185,9 +179,7 @@ TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigUnknownSecureFromEnv) unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT"); unsetenv("OTEL_EXPORTER_OTLP_METRICS_INSECURE"); } -# endif -# ifndef NO_GETENV // Test exporter configuration options with use_ssl_credentials TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigUnknownInsecureFromEnv) { @@ -202,9 +194,7 @@ TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigUnknownInsecureFromEnv) unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT"); unsetenv("OTEL_EXPORTER_OTLP_METRICS_INSECURE"); } -# endif -# ifndef NO_GETENV TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigRetryDefaultValues) { std::unique_ptr exporter(new OtlpGrpcMetricExporter()); From cb148574dfe99544c4d920ada5801b10374bb5bc Mon Sep 17 00:00:00 2001 From: Alex E Date: Sun, 29 Dec 2024 21:37:15 -0500 Subject: [PATCH 09/16] No real need for decimal seconds in otlp grpc exporter --- .../exporters/otlp/otlp_environment.h | 14 +++++------ .../exporters/otlp/otlp_grpc_client_options.h | 4 ++-- exporters/otlp/src/otlp_environment.cc | 24 +++++++++---------- exporters/otlp/src/otlp_grpc_client.cc | 11 ++++----- .../otlp/test/otlp_grpc_exporter_test.cc | 24 +++++++++---------- .../otlp_grpc_log_record_exporter_test.cc | 12 +++++----- .../test/otlp_grpc_metric_exporter_test.cc | 12 +++++----- 7 files changed, 48 insertions(+), 53 deletions(-) diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h index 799e07f27d..5eb1c58037 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h @@ -18,8 +18,6 @@ namespace exporter namespace otlp { -using SecondsDecimal = std::chrono::duration>; - inline std::string GetOtlpDefaultUserAgent() { return "OTel-OTLP-Exporter-Cpp/" OPENTELEMETRY_SDK_VERSION; @@ -158,13 +156,13 @@ std::uint32_t GetOtlpDefaultTracesRetryMaxAttempts(); std::uint32_t GetOtlpDefaultMetricsRetryMaxAttempts(); std::uint32_t GetOtlpDefaultLogsRetryMaxAttempts(); -SecondsDecimal GetOtlpDefaultTracesRetryInitialBackoff(); -SecondsDecimal GetOtlpDefaultMetricsRetryInitialBackoff(); -SecondsDecimal GetOtlpDefaultLogsRetryInitialBackoff(); +float GetOtlpDefaultTracesRetryInitialBackoff(); +float GetOtlpDefaultMetricsRetryInitialBackoff(); +float GetOtlpDefaultLogsRetryInitialBackoff(); -SecondsDecimal GetOtlpDefaultTracesRetryMaxBackoff(); -SecondsDecimal GetOtlpDefaultMetricsRetryMaxBackoff(); -SecondsDecimal GetOtlpDefaultLogsRetryMaxBackoff(); +float GetOtlpDefaultTracesRetryMaxBackoff(); +float GetOtlpDefaultMetricsRetryMaxBackoff(); +float GetOtlpDefaultLogsRetryMaxBackoff(); float GetOtlpDefaultTracesRetryBackoffMultiplier(); float GetOtlpDefaultMetricsRetryBackoffMultiplier(); diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client_options.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client_options.h index b1edbd9273..e092a9c586 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client_options.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client_options.h @@ -67,10 +67,10 @@ struct OtlpGrpcClientOptions std::uint32_t retry_policy_max_attempts{}; /** The initial backoff delay between retry attempts, random between (0, initial_backoff). */ - SecondsDecimal retry_policy_initial_backoff{}; + float retry_policy_initial_backoff{}; /** The maximum backoff places an upper limit on exponential backoff growth. */ - SecondsDecimal retry_policy_max_backoff{}; + float retry_policy_max_backoff{}; /** The backoff will be multiplied by this value after each retry attempt. */ float retry_policy_backoff_multiplier{}; diff --git a/exporters/otlp/src/otlp_environment.cc b/exporters/otlp/src/otlp_environment.cc index a9e43acb08..3fcc34c6ff 100644 --- a/exporters/otlp/src/otlp_environment.cc +++ b/exporters/otlp/src/otlp_environment.cc @@ -1174,46 +1174,46 @@ std::uint32_t GetOtlpDefaultLogsRetryMaxAttempts() return GetUintEnvVarOrDefault(kSignalEnv, kGenericEnv, 5U); } -SecondsDecimal GetOtlpDefaultTracesRetryInitialBackoff() +float GetOtlpDefaultTracesRetryInitialBackoff() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_RETRY_INITIAL_BACKOFF"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"; - return SecondsDecimal{GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 1.0)}; + return GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 1.0); } -SecondsDecimal GetOtlpDefaultMetricsRetryInitialBackoff() +float GetOtlpDefaultMetricsRetryInitialBackoff() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_RETRY_INITIAL_BACKOFF"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"; - return SecondsDecimal{GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 1.0)}; + return GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 1.0); } -SecondsDecimal GetOtlpDefaultLogsRetryInitialBackoff() +float GetOtlpDefaultLogsRetryInitialBackoff() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_RETRY_INITIAL_BACKOFF"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"; - return SecondsDecimal{GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 1.0)}; + return GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 1.0); } -SecondsDecimal GetOtlpDefaultTracesRetryMaxBackoff() +float GetOtlpDefaultTracesRetryMaxBackoff() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_RETRY_MAX_BACKOFF"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"; - return SecondsDecimal{GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 5.0)}; + return GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 5.0); } -SecondsDecimal GetOtlpDefaultMetricsRetryMaxBackoff() +float GetOtlpDefaultMetricsRetryMaxBackoff() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_RETRY_MAX_BACKOFF"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"; - return SecondsDecimal{GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 5.0)}; + return GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 5.0); } -SecondsDecimal GetOtlpDefaultLogsRetryMaxBackoff() +float GetOtlpDefaultLogsRetryMaxBackoff() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_RETRY_MAX_BACKOFF"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"; - return SecondsDecimal{GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 5.0)}; + return GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 5.0); } float GetOtlpDefaultTracesRetryBackoffMultiplier() diff --git a/exporters/otlp/src/otlp_grpc_client.cc b/exporters/otlp/src/otlp_grpc_client.cc index ec46129e95..7b73082cb7 100644 --- a/exporters/otlp/src/otlp_grpc_client.cc +++ b/exporters/otlp/src/otlp_grpc_client.cc @@ -349,10 +349,8 @@ std::shared_ptr OtlpGrpcClient::MakeChannel(const OtlpGrpcClientO grpc_arguments.SetCompressionAlgorithm(GRPC_COMPRESS_GZIP); } - if (options.retry_policy_max_attempts > 0U && - options.retry_policy_initial_backoff > SecondsDecimal::zero() && - options.retry_policy_max_backoff > SecondsDecimal::zero() && - options.retry_policy_backoff_multiplier > 0.f) + if (options.retry_policy_max_attempts > 0U && options.retry_policy_initial_backoff > 0.0f && + options.retry_policy_max_backoff > 0.0f && options.retry_policy_backoff_multiplier > 0.0f) { static const auto kServiceConfigJson = opentelemetry::nostd::string_view{R"( { @@ -382,9 +380,8 @@ std::shared_ptr OtlpGrpcClient::MakeChannel(const OtlpGrpcClientO // Prior to C++17, need to explicitly cast away constness from `data()` buffer std::snprintf(const_cast(service_config.data()), service_config.size(), kServiceConfigJson.data(), - options.retry_policy_max_attempts, options.retry_policy_initial_backoff.count(), - options.retry_policy_max_backoff.count(), - options.retry_policy_backoff_multiplier); + options.retry_policy_max_attempts, options.retry_policy_initial_backoff, + options.retry_policy_max_backoff, options.retry_policy_backoff_multiplier); grpc_arguments.SetServiceConfigJSON(service_config); } diff --git a/exporters/otlp/test/otlp_grpc_exporter_test.cc b/exporters/otlp/test/otlp_grpc_exporter_test.cc index 3588219c77..144dae20ff 100644 --- a/exporters/otlp/test/otlp_grpc_exporter_test.cc +++ b/exporters/otlp/test/otlp_grpc_exporter_test.cc @@ -359,8 +359,8 @@ TEST_F(OtlpGrpcExporterTestPeer, ConfigRetryDefaultValues) std::unique_ptr exporter(new OtlpGrpcExporter()); const auto options = GetOptions(exporter); ASSERT_EQ(options.retry_policy_max_attempts, 5); - ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 1); - ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 5); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff, 1.0); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff, 5.0); ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 1.5); } @@ -374,8 +374,8 @@ TEST_F(OtlpGrpcExporterTestPeer, ConfigRetryValuesFromEnv) std::unique_ptr exporter(new OtlpGrpcExporter()); const auto options = GetOptions(exporter); ASSERT_EQ(options.retry_policy_max_attempts, 123); - ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 4.5); - ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 6.7); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff, 4.5); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff, 6.7); ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 8.9); unsetenv("OTEL_EXPORTER_OTLP_TRACES_RETRY_MAX_ATTEMPTS"); @@ -394,8 +394,8 @@ TEST_F(OtlpGrpcExporterTestPeer, ConfigRetryGenericValuesFromEnv) std::unique_ptr exporter(new OtlpGrpcExporter()); const auto options = GetOptions(exporter); ASSERT_EQ(options.retry_policy_max_attempts, 321); - ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 5.4); - ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 7.6); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff, 5.4); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff, 7.6); ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 9.8); unsetenv("OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"); @@ -523,16 +523,16 @@ TEST_P(OtlpGrpcExporterRetryIntegrationTests, StatusCodes) if (is_retry_enabled) { opts.retry_policy_max_attempts = 5; - opts.retry_policy_initial_backoff = SecondsDecimal{0.1}; - opts.retry_policy_max_backoff = SecondsDecimal{5}; - opts.retry_policy_backoff_multiplier = 1; + opts.retry_policy_initial_backoff = 0.1f; + opts.retry_policy_max_backoff = 5.0f; + opts.retry_policy_backoff_multiplier = 1.0f; } else { opts.retry_policy_max_attempts = 0; - opts.retry_policy_initial_backoff = SecondsDecimal{0}; - opts.retry_policy_max_backoff = SecondsDecimal{0}; - opts.retry_policy_backoff_multiplier = 0; + opts.retry_policy_initial_backoff = 0.0f; + opts.retry_policy_max_backoff = 0.0f; + opts.retry_policy_backoff_multiplier = 0.0f; } auto exporter = otlp::OtlpGrpcExporterFactory::Create(opts); diff --git a/exporters/otlp/test/otlp_grpc_log_record_exporter_test.cc b/exporters/otlp/test/otlp_grpc_log_record_exporter_test.cc index 7f8bbada82..1dc9fd5b74 100644 --- a/exporters/otlp/test/otlp_grpc_log_record_exporter_test.cc +++ b/exporters/otlp/test/otlp_grpc_log_record_exporter_test.cc @@ -472,8 +472,8 @@ TEST_F(OtlpGrpcLogRecordExporterTestPeer, ConfigRetryDefaultValues) std::unique_ptr exporter(new OtlpGrpcLogRecordExporter()); const auto options = GetOptions(exporter); ASSERT_EQ(options.retry_policy_max_attempts, 5); - ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 1); - ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 5); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff, 1.0); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff, 5.0); ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 1.5); } @@ -487,8 +487,8 @@ TEST_F(OtlpGrpcLogRecordExporterTestPeer, ConfigRetryValuesFromEnv) std::unique_ptr exporter(new OtlpGrpcLogRecordExporter()); const auto options = GetOptions(exporter); ASSERT_EQ(options.retry_policy_max_attempts, 123); - ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 4.5); - ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 6.7); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff, 4.5); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff, 6.7); ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 8.9); unsetenv("OTEL_EXPORTER_OTLP_LOGS_RETRY_MAX_ATTEMPTS"); @@ -507,8 +507,8 @@ TEST_F(OtlpGrpcLogRecordExporterTestPeer, ConfigRetryGenericValuesFromEnv) std::unique_ptr exporter(new OtlpGrpcLogRecordExporter()); const auto options = GetOptions(exporter); ASSERT_EQ(options.retry_policy_max_attempts, 321); - ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 5.4); - ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 7.6); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff, 5.4); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff, 7.6); ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 9.8); unsetenv("OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"); diff --git a/exporters/otlp/test/otlp_grpc_metric_exporter_test.cc b/exporters/otlp/test/otlp_grpc_metric_exporter_test.cc index d0a73c1d14..661b4808b9 100644 --- a/exporters/otlp/test/otlp_grpc_metric_exporter_test.cc +++ b/exporters/otlp/test/otlp_grpc_metric_exporter_test.cc @@ -200,8 +200,8 @@ TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigRetryDefaultValues) std::unique_ptr exporter(new OtlpGrpcMetricExporter()); const auto options = GetOptions(exporter); ASSERT_EQ(options.retry_policy_max_attempts, 5); - ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 1); - ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 5); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff, 1.0); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff, 5.0); ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 1.5); } @@ -215,8 +215,8 @@ TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigRetryValuesFromEnv) std::unique_ptr exporter(new OtlpGrpcMetricExporter()); const auto options = GetOptions(exporter); ASSERT_EQ(options.retry_policy_max_attempts, 123); - ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 4.5); - ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 6.7); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff, 4.5); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff, 6.7); ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 8.9); unsetenv("OTEL_EXPORTER_OTLP_METRICS_RETRY_MAX_ATTEMPTS"); @@ -235,8 +235,8 @@ TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigRetryGenericValuesFromEnv) std::unique_ptr exporter(new OtlpGrpcMetricExporter()); const auto options = GetOptions(exporter); ASSERT_EQ(options.retry_policy_max_attempts, 321); - ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 5.4); - ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 7.6); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff, 5.4); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff, 7.6); ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 9.8); unsetenv("OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"); From 8677f8977159a03ff910bc3c9db94b3d710bbe06 Mon Sep 17 00:00:00 2001 From: Alex E Date: Tue, 7 Jan 2025 09:56:37 -0500 Subject: [PATCH 10/16] Code review feedback --- exporters/otlp/src/otlp_environment.cc | 140 +++++++++++--- .../otlp/test/otlp_grpc_exporter_test.cc | 2 +- .../opentelemetry/sdk/common/env_variables.h | 17 ++ sdk/src/common/env_variables.cc | 172 +++++++++++++++++- sdk/test/common/env_var_test.cc | 125 ++++++++++++- 5 files changed, 418 insertions(+), 38 deletions(-) diff --git a/exporters/otlp/src/otlp_environment.cc b/exporters/otlp/src/otlp_environment.cc index 3fcc34c6ff..22efeb93bb 100644 --- a/exporters/otlp/src/otlp_environment.cc +++ b/exporters/otlp/src/otlp_environment.cc @@ -80,32 +80,36 @@ static bool GetStringDualEnvVar(const char *signal_name, return exists; } -static std::uint32_t GetUintEnvVarOrDefault(opentelemetry::nostd::string_view signal_env, - opentelemetry::nostd::string_view generic_env, - std::uint32_t default_value) +static bool GetUintDualEnvVar(const char *signal_name, + const char *generic_name, + std::uint32_t &value) { - std::string value; + bool exists; - if (GetStringDualEnvVar(signal_env.data(), generic_env.data(), value)) + exists = sdk_common::GetUintEnvironmentVariable(signal_name, value); + if (exists) { - return static_cast(std::strtoul(value.c_str(), nullptr, 10)); + return true; } - return default_value; + exists = sdk_common::GetUintEnvironmentVariable(generic_name, value); + + return exists; } -static float GetFloatEnvVarOrDefault(opentelemetry::nostd::string_view signal_env, - opentelemetry::nostd::string_view generic_env, - float default_value) +static float GetFloatDualEnvVar(const char *signal_name, const char *generic_name, float &value) { - std::string value; + bool exists; - if (GetStringDualEnvVar(signal_env.data(), generic_env.data(), value)) + exists = sdk_common::GetFloatEnvironmentVariable(signal_name, value); + if (exists) { - return std::strtof(value.c_str(), nullptr); + return true; } - return default_value; + exists = sdk_common::GetFloatEnvironmentVariable(generic_name, value); + + return exists; } std::string GetOtlpDefaultGrpcTracesEndpoint() @@ -1157,84 +1161,168 @@ std::uint32_t GetOtlpDefaultTracesRetryMaxAttempts() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_RETRY_MAX_ATTEMPTS"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"; - return GetUintEnvVarOrDefault(kSignalEnv, kGenericEnv, 5U); + std::uint32_t value{}; + + if (GetUintDualEnvVar(kSignalEnv, kGenericEnv, value)) + { + return value; + } + + return 5U; } std::uint32_t GetOtlpDefaultMetricsRetryMaxAttempts() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_RETRY_MAX_ATTEMPTS"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"; - return GetUintEnvVarOrDefault(kSignalEnv, kGenericEnv, 5U); + std::uint32_t value{}; + + if (GetUintDualEnvVar(kSignalEnv, kGenericEnv, value)) + { + return value; + } + + return 5U; } std::uint32_t GetOtlpDefaultLogsRetryMaxAttempts() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_RETRY_MAX_ATTEMPTS"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"; - return GetUintEnvVarOrDefault(kSignalEnv, kGenericEnv, 5U); + std::uint32_t value{}; + + if (GetUintDualEnvVar(kSignalEnv, kGenericEnv, value)) + { + return value; + } + + return 5U; } float GetOtlpDefaultTracesRetryInitialBackoff() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_RETRY_INITIAL_BACKOFF"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"; - return GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 1.0); + float value{}; + + if (GetFloatDualEnvVar(kSignalEnv, kGenericEnv, value)) + { + return value; + } + + return 1.0f; } float GetOtlpDefaultMetricsRetryInitialBackoff() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_RETRY_INITIAL_BACKOFF"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"; - return GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 1.0); + float value{}; + + if (GetFloatDualEnvVar(kSignalEnv, kGenericEnv, value)) + { + return value; + } + + return 1.0f; } float GetOtlpDefaultLogsRetryInitialBackoff() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_RETRY_INITIAL_BACKOFF"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"; - return GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 1.0); + float value{}; + + if (GetFloatDualEnvVar(kSignalEnv, kGenericEnv, value)) + { + return value; + } + + return 1.0f; } float GetOtlpDefaultTracesRetryMaxBackoff() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_RETRY_MAX_BACKOFF"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"; - return GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 5.0); + float value{}; + + if (GetFloatDualEnvVar(kSignalEnv, kGenericEnv, value)) + { + return value; + } + + return 5.0f; } float GetOtlpDefaultMetricsRetryMaxBackoff() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_RETRY_MAX_BACKOFF"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"; - return GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 5.0); + float value{}; + + if (GetFloatDualEnvVar(kSignalEnv, kGenericEnv, value)) + { + return value; + } + + return 5.0f; } float GetOtlpDefaultLogsRetryMaxBackoff() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_RETRY_MAX_BACKOFF"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"; - return GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 5.0); + float value{}; + + if (GetFloatDualEnvVar(kSignalEnv, kGenericEnv, value)) + { + return value; + } + + return 5.0f; } float GetOtlpDefaultTracesRetryBackoffMultiplier() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_RETRY_BACKOFF_MULTIPLIER"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"; - return GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 1.5f); + float value{}; + + if (GetFloatDualEnvVar(kSignalEnv, kGenericEnv, value)) + { + return value; + } + + return 1.5f; } float GetOtlpDefaultMetricsRetryBackoffMultiplier() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_RETRY_BACKOFF_MULTIPLIER"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"; - return GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 1.5f); + float value{}; + + if (GetFloatDualEnvVar(kSignalEnv, kGenericEnv, value)) + { + return value; + } + + return 1.5f; } float GetOtlpDefaultLogsRetryBackoffMultiplier() { constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_RETRY_BACKOFF_MULTIPLIER"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"; - return GetFloatEnvVarOrDefault(kSignalEnv, kGenericEnv, 1.5f); + float value{}; + + if (GetFloatDualEnvVar(kSignalEnv, kGenericEnv, value)) + { + return value; + } + + return 1.5f; } } // namespace otlp diff --git a/exporters/otlp/test/otlp_grpc_exporter_test.cc b/exporters/otlp/test/otlp_grpc_exporter_test.cc index 144dae20ff..be1ddbb3cf 100644 --- a/exporters/otlp/test/otlp_grpc_exporter_test.cc +++ b/exporters/otlp/test/otlp_grpc_exporter_test.cc @@ -19,7 +19,6 @@ # include "opentelemetry/exporters/otlp/otlp_grpc_exporter.h" # include "opentelemetry/exporters/otlp/otlp_grpc_exporter_factory.h" # include "opentelemetry/exporters/otlp/protobuf_include_prefix.h" -# include "opentelemetry/nostd/shared_ptr.h" // Problematic code that pulls in Gmock and breaks with vs2019/c++latest : # include "opentelemetry/proto/collector/trace/v1/trace_service_mock.grpc.pb.h" @@ -28,6 +27,7 @@ # include "opentelemetry/exporters/otlp/protobuf_include_suffix.h" +# include "opentelemetry/nostd/shared_ptr.h" # include "opentelemetry/sdk/trace/simple_processor.h" # include "opentelemetry/sdk/trace/simple_processor_factory.h" # include "opentelemetry/sdk/trace/tracer_provider.h" diff --git a/sdk/include/opentelemetry/sdk/common/env_variables.h b/sdk/include/opentelemetry/sdk/common/env_variables.h index a02a66c29e..7bf28e2c1c 100644 --- a/sdk/include/opentelemetry/sdk/common/env_variables.h +++ b/sdk/include/opentelemetry/sdk/common/env_variables.h @@ -4,6 +4,7 @@ #pragma once #include +#include #include #include "opentelemetry/version.h" @@ -39,6 +40,22 @@ bool GetDurationEnvironmentVariable(const char *env_var_name, */ bool GetStringEnvironmentVariable(const char *env_var_name, std::string &value); +/** + Read a uint32_t environment variable. + @param env_var_name Environment variable name + @param [out] value Variable value, if it exists + @return true if the variable exists +*/ +bool GetUintEnvironmentVariable(const char *env_var_name, std::uint32_t &value); + +/** + Read a float environment variable. + @param env_var_name Environment variable name + @param [out] value Variable value, if it exists + @return true if the variable exists +*/ +bool GetFloatEnvironmentVariable(const char *env_var_name, float &value); + #if defined(_MSC_VER) inline int setenv(const char *name, const char *value, int) { diff --git a/sdk/src/common/env_variables.cc b/sdk/src/common/env_variables.cc index 586a8ed420..2d9c63278f 100644 --- a/sdk/src/common/env_variables.cc +++ b/sdk/src/common/env_variables.cc @@ -10,8 +10,12 @@ # include #endif +#include +#include #include +#include #include +#include #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/common/global_log_handler.h" @@ -23,6 +27,117 @@ namespace sdk namespace common { +template ::value, bool> = true> +constexpr bool ParseNumber(T &&input, U &output) +{ + constexpr auto max_value = std::numeric_limits::max(); + + // Skip spaces + for (; *input && std::isspace(*input); ++input) + ; + + const auto is_negative = (*input && *input == '-'); + + if (is_negative) + { + ++input; + + // Reject negative when expecting unsigned + if (std::is_unsigned::value) + { + return false; + } + } + + const auto pos = input; + U result = 0; + U temp = 0; + + for (; *input && std::isdigit(*input); ++input) + { + temp = result * 10 + (*input - '0'); + + if (max_value - temp > max_value - result) + { + return false; // Overflow protection + } + + result = temp; + } + + result *= (is_negative) ? -1 : 1; + + // Reject if nothing parsed + if (pos != input) + { + output = result; + return true; + } + + return false; +} + +template ::value, bool> = true> +constexpr bool ParseNumber(T &&input, U &output) +{ + // Skip spaces + for (; *input && std::isspace(*input); ++input) + ; + + const auto is_negative = (*input && *input == '-'); + + if (is_negative) + { + ++input; + } + + const auto pos = input; + U result = 0; + U temp = 0; + U decimal_mul = 0.1f; + bool is_decimal = false; + + for (; *input && (std::isdigit(*input) || !is_decimal); ++input) + { + if (*input == '.' && !is_decimal) + { + is_decimal = true; + continue; + } + else if (*input == '.' && is_decimal) + { + break; + } + else if (is_decimal) + { + temp = result + decimal_mul * (*input - '0'); + decimal_mul *= 0.1f; + } + else + { + temp = result * 10 + (*input - '0'); + } + + if (std::isinf(temp)) + { + return false; // Overflow protection + } + + result = temp; + } + + result *= (is_negative) ? -1.0f : 1.0f; + + // Reject if nothing parsed + if (pos != input && !std::isnan(output) && !std::isinf(output)) + { + output = result; + return true; + } + + return false; +} + bool GetRawEnvironmentVariable(const char *env_var_name, std::string &value) { #if !defined(NO_GETENV) @@ -88,16 +203,7 @@ static bool GetTimeoutFromString(const char *input, std::chrono::system_clock::d { std::chrono::system_clock::duration::rep result = 0; - // Skip spaces - for (; *input && (' ' == *input || '\t' == *input || '\r' == *input || '\n' == *input); ++input) - ; - - for (; *input && (*input >= '0' && *input <= '9'); ++input) - { - result = result * 10 + (*input - '0'); - } - - if (result == 0) + if (!ParseNumber(input, result)) { // Rejecting duration 0 as invalid. return false; @@ -193,6 +299,52 @@ bool GetStringEnvironmentVariable(const char *env_var_name, std::string &value) return true; } +bool GetUintEnvironmentVariable(const char *env_var_name, std::uint32_t &value) +{ + static constexpr auto kDefaultValue = 0U; + std::string raw_value; + bool exists = GetRawEnvironmentVariable(env_var_name, raw_value); + if (!exists || raw_value.empty()) + { + value = kDefaultValue; + return false; + } + + if (!ParseNumber(raw_value.c_str(), value)) + { + OTEL_INTERNAL_LOG_WARN("Environment variable <" << env_var_name << "> has an invalid value <" + << raw_value << ">, defaulting to " + << kDefaultValue); + value = kDefaultValue; + return false; + } + + return true; +} + +bool GetFloatEnvironmentVariable(const char *env_var_name, float &value) +{ + static constexpr auto kDefaultValue = 0.0f; + std::string raw_value; + bool exists = GetRawEnvironmentVariable(env_var_name, raw_value); + if (!exists || raw_value.empty()) + { + value = kDefaultValue; + return false; + } + + if (!ParseNumber(raw_value.c_str(), value)) + { + OTEL_INTERNAL_LOG_WARN("Environment variable <" << env_var_name << "> has an invalid value <" + << raw_value << ">, defaulting to " + << kDefaultValue); + value = kDefaultValue; + return false; + } + + return true; +} + } // namespace common } // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/test/common/env_var_test.cc b/sdk/test/common/env_var_test.cc index 8f64dc1f10..d529f3fcf9 100644 --- a/sdk/test/common/env_var_test.cc +++ b/sdk/test/common/env_var_test.cc @@ -15,7 +15,9 @@ using opentelemetry::sdk::common::unsetenv; using opentelemetry::sdk::common::GetBoolEnvironmentVariable; using opentelemetry::sdk::common::GetDurationEnvironmentVariable; +using opentelemetry::sdk::common::GetFloatEnvironmentVariable; using opentelemetry::sdk::common::GetStringEnvironmentVariable; +using opentelemetry::sdk::common::GetUintEnvironmentVariable; #ifndef NO_GETENV TEST(EnvVarTest, BoolEnvVar) @@ -210,4 +212,125 @@ TEST(EnvVarTest, DurationEnvVar) unsetenv("STRING_ENV_VAR_BROKEN_2"); } -#endif +TEST(EnvVarTest, UintEnvVar) +{ + unsetenv("UINT_ENV_VAR_NONE"); + setenv("UINT_ENV_VAR_EMPTY", "", 1); + setenv("UINT_ENV_VAR_POSITIVE_INT", "42", 1); + setenv("UINT_ENV_VAR_NEGATIVE_INT", "-42", 1); + setenv("UINT_ENV_VAR_POSITIVE_DEC", "12.34", 1); + setenv("UINT_ENV_VAR_NEGATIVE_DEC", "-12.34", 1); + setenv("UINT_ENV_VAR_POSITIVE_INT_MAX", "4294967295", 1); + setenv("UINT_ENV_VAR_POSITIVE_OVERFLOW", "4294967296", 1); + setenv("UINT_ENV_VAR_NEGATIVE_INT_MIN", "-2147483648", 1); + setenv("UINT_ENV_VAR_NEGATIVE_OVERFLOW", "-4294967296", 1); + setenv("UINT_ENV_VAR_WITH_NOISE", " \t \n 9.12345678.9", 1); + setenv("UINT_ENV_VAR_ONLY_SPACES", " ", 1); + + std::uint32_t value; + + ASSERT_FALSE(GetUintEnvironmentVariable("UINT_ENV_VAR_NONE", value)); + + ASSERT_FALSE(GetUintEnvironmentVariable("UINT_ENV_VAR_EMPTY", value)); + + ASSERT_TRUE(GetUintEnvironmentVariable("UINT_ENV_VAR_POSITIVE_INT", value)); + ASSERT_EQ(42, value); + + ASSERT_FALSE(GetUintEnvironmentVariable("UINT_ENV_VAR_NEGATIVE_INT", value)); + + ASSERT_TRUE(GetUintEnvironmentVariable("UINT_ENV_VAR_POSITIVE_DEC", value)); + ASSERT_EQ(12, value); + + ASSERT_FALSE(GetUintEnvironmentVariable("UINT_ENV_VAR_NEGATIVE_DEC", value)); + + ASSERT_TRUE(GetUintEnvironmentVariable("UINT_ENV_VAR_POSITIVE_INT_MAX", value)); + ASSERT_EQ(4294967295, value); + + ASSERT_FALSE(GetUintEnvironmentVariable("UINT_ENV_VAR_POSITIVE_OVERFLOW", value)); + + ASSERT_FALSE(GetUintEnvironmentVariable("UINT_ENV_VAR_NEGATIVE_INT_MIN", value)); + + ASSERT_FALSE(GetUintEnvironmentVariable("UINT_ENV_VAR_NEGATIVE_OVERFLOW", value)); + + ASSERT_TRUE(GetUintEnvironmentVariable("UINT_ENV_VAR_WITH_NOISE", value)); + ASSERT_EQ(9, value); + + ASSERT_FALSE(GetUintEnvironmentVariable("UINT_ENV_VAR_ONLY_SPACES", value)); + + unsetenv("UINT_ENV_VAR_EMPTY"); + unsetenv("UINT_ENV_VAR_POSITIVE_INT"); + unsetenv("UINT_ENV_VAR_NEGATIVE_INT"); + unsetenv("UINT_ENV_VAR_POSITIVE_DEC"); + unsetenv("UINT_ENV_VAR_NEGATIVE_DEC"); + unsetenv("UINT_ENV_VAR_POSITIVE_INT_MAX"); + unsetenv("UINT_ENV_VAR_POSITIVE_OVERFLOW"); + unsetenv("UINT_ENV_VAR_NEGATIVE_INT_MIN"); + unsetenv("UINT_ENV_VAR_NEGATIVE_OVERFLOW"); + unsetenv("UINT_ENV_VAR_WITH_NOISE"); + unsetenv("UINT_ENV_VAR_ONLY_SPACES"); +} + +TEST(EnvVarTest, FloatEnvVar) +{ + unsetenv("FLOAT_ENV_VAR_NONE"); + setenv("FLOAT_ENV_VAR_EMPTY", "", 1); + setenv("FLOAT_ENV_VAR_POSITIVE_INT", "42", 1); + setenv("FLOAT_ENV_VAR_NEGATIVE_INT", "-42", 1); + setenv("FLOAT_ENV_VAR_POSITIVE_DEC", "12.34", 1); + setenv("FLOAT_ENV_VAR_NEGATIVE_DEC", "-12.34", 1); + setenv("FLOAT_ENV_VAR_POSITIVE_INT_MAX", "4294967295", 1); + setenv("FLOAT_ENV_VAR_POSITIVE_OVERFLOW", "4294967296", 1); + setenv("FLOAT_ENV_VAR_NEGATIVE_INT_MIN", "-2147483648", 1); + setenv("FLOAT_ENV_VAR_NEGATIVE_OVERFLOW", "-4294967296", 1); + setenv("FLOAT_ENV_VAR_WITH_NOISE", " \t \n 9.12345678.9", 1); + setenv("FLOAT_ENV_VAR_ONLY_SPACES", " ", 1); + + float value; + + ASSERT_FALSE(GetFloatEnvironmentVariable("FLOAT_ENV_VAR_NONE", value)); + + ASSERT_FALSE(GetFloatEnvironmentVariable("FLOAT_ENV_VAR_EMPTY", value)); + + ASSERT_TRUE(GetFloatEnvironmentVariable("FLOAT_ENV_VAR_POSITIVE_INT", value)); + ASSERT_FLOAT_EQ(42.f, value); + + ASSERT_TRUE(GetFloatEnvironmentVariable("FLOAT_ENV_VAR_NEGATIVE_INT", value)); + ASSERT_FLOAT_EQ(-42.f, value); + + ASSERT_TRUE(GetFloatEnvironmentVariable("FLOAT_ENV_VAR_POSITIVE_DEC", value)); + ASSERT_FLOAT_EQ(12.34f, value); + + ASSERT_TRUE(GetFloatEnvironmentVariable("FLOAT_ENV_VAR_NEGATIVE_DEC", value)); + ASSERT_FLOAT_EQ(-12.34f, value); + + ASSERT_TRUE(GetFloatEnvironmentVariable("FLOAT_ENV_VAR_POSITIVE_INT_MAX", value)); + ASSERT_FLOAT_EQ(4294967295.f, value); + + ASSERT_TRUE(GetFloatEnvironmentVariable("FLOAT_ENV_VAR_POSITIVE_OVERFLOW", value)); + ASSERT_FLOAT_EQ(4294967296.f, value); + + ASSERT_TRUE(GetFloatEnvironmentVariable("FLOAT_ENV_VAR_NEGATIVE_INT_MIN", value)); + ASSERT_FLOAT_EQ(-2147483648.f, value); + + ASSERT_TRUE(GetFloatEnvironmentVariable("FLOAT_ENV_VAR_NEGATIVE_OVERFLOW", value)); + ASSERT_FLOAT_EQ(-4294967296.f, value); + + ASSERT_TRUE(GetFloatEnvironmentVariable("FLOAT_ENV_VAR_WITH_NOISE", value)); + ASSERT_FLOAT_EQ(9.12345678f, value); + + ASSERT_FALSE(GetFloatEnvironmentVariable("FLOAT_ENV_VAR_ONLY_SPACES", value)); + + unsetenv("FLOAT_ENV_VAR_EMPTY"); + unsetenv("FLOAT_ENV_VAR_POSITIVE_INT"); + unsetenv("FLOAT_ENV_VAR_NEGATIVE_INT"); + unsetenv("FLOAT_ENV_VAR_POSITIVE_DEC"); + unsetenv("FLOAT_ENV_VAR_NEGATIVE_DEC"); + unsetenv("FLOAT_ENV_VAR_POSITIVE_INT_MAX"); + unsetenv("FLOAT_ENV_VAR_POSITIVE_OVERFLOW"); + unsetenv("FLOAT_ENV_VAR_NEGATIVE_INT_MIN"); + unsetenv("FLOAT_ENV_VAR_NEGATIVE_OVERFLOW"); + unsetenv("FLOAT_ENV_VAR_WITH_NOISE"); + unsetenv("FLOAT_ENV_VAR_ONLY_SPACES"); +} + +#endif // NO_GETENV From 589606aaa4e5a159907200cecfbdd9f0efe189a4 Mon Sep 17 00:00:00 2001 From: Alex E Date: Thu, 9 Jan 2025 22:25:18 -0500 Subject: [PATCH 11/16] Cherry pick common bits from PR for OTLP HTTP retries --- CMakeLists.txt | 3 + api/CMakeLists.txt | 5 + ci/do_ci.sh | 4 + .../exporters/otlp/otlp_environment.h | 12 +- .../exporters/otlp/otlp_grpc_client_options.h | 4 +- exporters/otlp/src/otlp_environment.cc | 87 ++++----- sdk/src/common/env_variables.cc | 172 +++++------------- sdk/test/common/env_var_test.cc | 30 ++- 8 files changed, 137 insertions(+), 180 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 408ee43046..9448ff5728 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -212,6 +212,9 @@ if(NOT WITH_STL STREQUAL "OFF") endif() endif() +option(WITH_OTLP_RETRY_PREVIEW + "Whether to enable experimental retry functionality" OFF) + option(WITH_OTLP_GRPC_SSL_MTLS_PREVIEW "Whether to enable mTLS support fro gRPC" OFF) diff --git a/api/CMakeLists.txt b/api/CMakeLists.txt index 78e97ad029..65a123293d 100644 --- a/api/CMakeLists.txt +++ b/api/CMakeLists.txt @@ -116,6 +116,11 @@ target_compile_definitions( opentelemetry_api INTERFACE OPENTELEMETRY_ABI_VERSION_NO=${OPENTELEMETRY_ABI_VERSION_NO}) +if(WITH_OTLP_RETRY_PREVIEW) + target_compile_definitions(opentelemetry_api + INTERFACE ENABLE_OTLP_RETRY_PREVIEW) +endif() + if(WITH_OTLP_GRPC_SSL_MTLS_PREVIEW) target_compile_definitions(opentelemetry_api INTERFACE ENABLE_OTLP_GRPC_SSL_MTLS_PREVIEW) diff --git a/ci/do_ci.sh b/ci/do_ci.sh index 11dbe4ef80..0f6add0ff5 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -131,6 +131,7 @@ elif [[ "$1" == "cmake.maintainer.sync.test" ]]; then -DOTELCPP_MAINTAINER_MODE=ON \ -DWITH_NO_DEPRECATED_CODE=ON \ -DWITH_OTLP_HTTP_COMPRESSION=ON \ + -DWITH_OTLP_RETRY_PREVIEW=ON \ ${IWYU} \ "${SRC_DIR}" eval "$MAKE_COMMAND" @@ -153,6 +154,7 @@ elif [[ "$1" == "cmake.maintainer.async.test" ]]; then -DOTELCPP_MAINTAINER_MODE=ON \ -DWITH_NO_DEPRECATED_CODE=ON \ -DWITH_OTLP_HTTP_COMPRESSION=ON \ + -DWITH_OTLP_RETRY_PREVIEW=ON \ ${IWYU} \ "${SRC_DIR}" eval "$MAKE_COMMAND" @@ -176,6 +178,7 @@ elif [[ "$1" == "cmake.maintainer.cpp11.async.test" ]]; then -DOTELCPP_MAINTAINER_MODE=ON \ -DWITH_NO_DEPRECATED_CODE=ON \ -DWITH_OTLP_HTTP_COMPRESSION=ON \ + -DWITH_OTLP_RETRY_PREVIEW=ON \ "${SRC_DIR}" make -k -j $(nproc) make test @@ -199,6 +202,7 @@ elif [[ "$1" == "cmake.maintainer.abiv2.test" ]]; then -DWITH_ABI_VERSION_1=OFF \ -DWITH_ABI_VERSION_2=ON \ -DWITH_OTLP_HTTP_COMPRESSION=ON \ + -DWITH_OTLP_RETRY_PREVIEW=ON \ ${IWYU} \ "${SRC_DIR}" eval "$MAKE_COMMAND" diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h index 5eb1c58037..f6ea55d32d 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h @@ -156,13 +156,13 @@ std::uint32_t GetOtlpDefaultTracesRetryMaxAttempts(); std::uint32_t GetOtlpDefaultMetricsRetryMaxAttempts(); std::uint32_t GetOtlpDefaultLogsRetryMaxAttempts(); -float GetOtlpDefaultTracesRetryInitialBackoff(); -float GetOtlpDefaultMetricsRetryInitialBackoff(); -float GetOtlpDefaultLogsRetryInitialBackoff(); +std::chrono::duration GetOtlpDefaultTracesRetryInitialBackoff(); +std::chrono::duration GetOtlpDefaultMetricsRetryInitialBackoff(); +std::chrono::duration GetOtlpDefaultLogsRetryInitialBackoff(); -float GetOtlpDefaultTracesRetryMaxBackoff(); -float GetOtlpDefaultMetricsRetryMaxBackoff(); -float GetOtlpDefaultLogsRetryMaxBackoff(); +std::chrono::duration GetOtlpDefaultTracesRetryMaxBackoff(); +std::chrono::duration GetOtlpDefaultMetricsRetryMaxBackoff(); +std::chrono::duration GetOtlpDefaultLogsRetryMaxBackoff(); float GetOtlpDefaultTracesRetryBackoffMultiplier(); float GetOtlpDefaultMetricsRetryBackoffMultiplier(); diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client_options.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client_options.h index e092a9c586..babb4cac54 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client_options.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client_options.h @@ -67,10 +67,10 @@ struct OtlpGrpcClientOptions std::uint32_t retry_policy_max_attempts{}; /** The initial backoff delay between retry attempts, random between (0, initial_backoff). */ - float retry_policy_initial_backoff{}; + std::chrono::duration retry_policy_initial_backoff{}; /** The maximum backoff places an upper limit on exponential backoff growth. */ - float retry_policy_max_backoff{}; + std::chrono::duration retry_policy_max_backoff{}; /** The backoff will be multiplied by this value after each retry attempt. */ float retry_policy_backoff_multiplier{}; diff --git a/exporters/otlp/src/otlp_environment.cc b/exporters/otlp/src/otlp_environment.cc index 6be2d04615..a7bc95b321 100644 --- a/exporters/otlp/src/otlp_environment.cc +++ b/exporters/otlp/src/otlp_environment.cc @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 #include +#include #include #include #include @@ -95,7 +96,7 @@ static bool GetUintDualEnvVar(const char *signal_name, return exists; } -static float GetFloatDualEnvVar(const char *signal_name, const char *generic_name, float &value) +static bool GetFloatDualEnvVar(const char *signal_name, const char *generic_name, float &value) { bool exists; @@ -1157,8 +1158,8 @@ std::string GetOtlpDefaultLogsCompression() std::uint32_t GetOtlpDefaultTracesRetryMaxAttempts() { - constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_RETRY_MAX_ATTEMPTS"; - constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"; + constexpr char kSignalEnv[] = "OTEL_CPP_EXPORTER_OTLP_TRACES_RETRY_MAX_ATTEMPTS"; + constexpr char kGenericEnv[] = "OTEL_CPP_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"; std::uint32_t value{}; if (GetUintDualEnvVar(kSignalEnv, kGenericEnv, value)) @@ -1171,8 +1172,8 @@ std::uint32_t GetOtlpDefaultTracesRetryMaxAttempts() std::uint32_t GetOtlpDefaultMetricsRetryMaxAttempts() { - constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_RETRY_MAX_ATTEMPTS"; - constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"; + constexpr char kSignalEnv[] = "OTEL_CPP_EXPORTER_OTLP_METRICS_RETRY_MAX_ATTEMPTS"; + constexpr char kGenericEnv[] = "OTEL_CPP_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"; std::uint32_t value{}; if (GetUintDualEnvVar(kSignalEnv, kGenericEnv, value)) @@ -1185,8 +1186,8 @@ std::uint32_t GetOtlpDefaultMetricsRetryMaxAttempts() std::uint32_t GetOtlpDefaultLogsRetryMaxAttempts() { - constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_RETRY_MAX_ATTEMPTS"; - constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"; + constexpr char kSignalEnv[] = "OTEL_CPP_EXPORTER_OTLP_LOGS_RETRY_MAX_ATTEMPTS"; + constexpr char kGenericEnv[] = "OTEL_CPP_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"; std::uint32_t value{}; if (GetUintDualEnvVar(kSignalEnv, kGenericEnv, value)) @@ -1197,94 +1198,94 @@ std::uint32_t GetOtlpDefaultLogsRetryMaxAttempts() return 5U; } -float GetOtlpDefaultTracesRetryInitialBackoff() +std::chrono::duration GetOtlpDefaultTracesRetryInitialBackoff() { - constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_RETRY_INITIAL_BACKOFF"; - constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"; + constexpr char kSignalEnv[] = "OTEL_CPP_EXPORTER_OTLP_TRACES_RETRY_INITIAL_BACKOFF"; + constexpr char kGenericEnv[] = "OTEL_CPP_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"; float value{}; if (GetFloatDualEnvVar(kSignalEnv, kGenericEnv, value)) { - return value; + return std::chrono::duration{value}; } - return 1.0f; + return std::chrono::duration{1.0f}; } -float GetOtlpDefaultMetricsRetryInitialBackoff() +std::chrono::duration GetOtlpDefaultMetricsRetryInitialBackoff() { - constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_RETRY_INITIAL_BACKOFF"; - constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"; + constexpr char kSignalEnv[] = "OTEL_CPP_EXPORTER_OTLP_METRICS_RETRY_INITIAL_BACKOFF"; + constexpr char kGenericEnv[] = "OTEL_CPP_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"; float value{}; if (GetFloatDualEnvVar(kSignalEnv, kGenericEnv, value)) { - return value; + return std::chrono::duration{value}; } - return 1.0f; + return std::chrono::duration{1.0f}; } -float GetOtlpDefaultLogsRetryInitialBackoff() +std::chrono::duration GetOtlpDefaultLogsRetryInitialBackoff() { - constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_RETRY_INITIAL_BACKOFF"; - constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"; + constexpr char kSignalEnv[] = "OTEL_CPP_EXPORTER_OTLP_LOGS_RETRY_INITIAL_BACKOFF"; + constexpr char kGenericEnv[] = "OTEL_CPP_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"; float value{}; if (GetFloatDualEnvVar(kSignalEnv, kGenericEnv, value)) { - return value; + return std::chrono::duration{value}; } - return 1.0f; + return std::chrono::duration{1.0f}; } -float GetOtlpDefaultTracesRetryMaxBackoff() +std::chrono::duration GetOtlpDefaultTracesRetryMaxBackoff() { - constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_RETRY_MAX_BACKOFF"; - constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"; + constexpr char kSignalEnv[] = "OTEL_CPP_EXPORTER_OTLP_TRACES_RETRY_MAX_BACKOFF"; + constexpr char kGenericEnv[] = "OTEL_CPP_EXPORTER_OTLP_RETRY_MAX_BACKOFF"; float value{}; if (GetFloatDualEnvVar(kSignalEnv, kGenericEnv, value)) { - return value; + return std::chrono::duration{value}; } - return 5.0f; + return std::chrono::duration{5.0f}; } -float GetOtlpDefaultMetricsRetryMaxBackoff() +std::chrono::duration GetOtlpDefaultMetricsRetryMaxBackoff() { - constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_RETRY_MAX_BACKOFF"; - constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"; + constexpr char kSignalEnv[] = "OTEL_CPP_EXPORTER_OTLP_METRICS_RETRY_MAX_BACKOFF"; + constexpr char kGenericEnv[] = "OTEL_CPP_EXPORTER_OTLP_RETRY_MAX_BACKOFF"; float value{}; if (GetFloatDualEnvVar(kSignalEnv, kGenericEnv, value)) { - return value; + return std::chrono::duration{value}; } - return 5.0f; + return std::chrono::duration{5.0f}; } -float GetOtlpDefaultLogsRetryMaxBackoff() +std::chrono::duration GetOtlpDefaultLogsRetryMaxBackoff() { - constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_RETRY_MAX_BACKOFF"; - constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"; + constexpr char kSignalEnv[] = "OTEL_CPP_EXPORTER_OTLP_LOGS_RETRY_MAX_BACKOFF"; + constexpr char kGenericEnv[] = "OTEL_CPP_EXPORTER_OTLP_RETRY_MAX_BACKOFF"; float value{}; if (GetFloatDualEnvVar(kSignalEnv, kGenericEnv, value)) { - return value; + return std::chrono::duration{value}; } - return 5.0f; + return std::chrono::duration{5.0f}; } float GetOtlpDefaultTracesRetryBackoffMultiplier() { - constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_RETRY_BACKOFF_MULTIPLIER"; - constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"; + constexpr char kSignalEnv[] = "OTEL_CPP_EXPORTER_OTLP_TRACES_RETRY_BACKOFF_MULTIPLIER"; + constexpr char kGenericEnv[] = "OTEL_CPP_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"; float value{}; if (GetFloatDualEnvVar(kSignalEnv, kGenericEnv, value)) @@ -1297,8 +1298,8 @@ float GetOtlpDefaultTracesRetryBackoffMultiplier() float GetOtlpDefaultMetricsRetryBackoffMultiplier() { - constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_RETRY_BACKOFF_MULTIPLIER"; - constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"; + constexpr char kSignalEnv[] = "OTEL_CPP_EXPORTER_OTLP_METRICS_RETRY_BACKOFF_MULTIPLIER"; + constexpr char kGenericEnv[] = "OTEL_CPP_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"; float value{}; if (GetFloatDualEnvVar(kSignalEnv, kGenericEnv, value)) @@ -1311,8 +1312,8 @@ float GetOtlpDefaultMetricsRetryBackoffMultiplier() float GetOtlpDefaultLogsRetryBackoffMultiplier() { - constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_RETRY_BACKOFF_MULTIPLIER"; - constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"; + constexpr char kSignalEnv[] = "OTEL_CPP_EXPORTER_OTLP_LOGS_RETRY_BACKOFF_MULTIPLIER"; + constexpr char kGenericEnv[] = "OTEL_CPP_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"; float value{}; if (GetFloatDualEnvVar(kSignalEnv, kGenericEnv, value)) diff --git a/sdk/src/common/env_variables.cc b/sdk/src/common/env_variables.cc index 2d9c63278f..016f738d4e 100644 --- a/sdk/src/common/env_variables.cc +++ b/sdk/src/common/env_variables.cc @@ -11,11 +11,10 @@ #endif #include -#include +#include #include #include #include -#include #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/common/global_log_handler.h" @@ -27,117 +26,6 @@ namespace sdk namespace common { -template ::value, bool> = true> -constexpr bool ParseNumber(T &&input, U &output) -{ - constexpr auto max_value = std::numeric_limits::max(); - - // Skip spaces - for (; *input && std::isspace(*input); ++input) - ; - - const auto is_negative = (*input && *input == '-'); - - if (is_negative) - { - ++input; - - // Reject negative when expecting unsigned - if (std::is_unsigned::value) - { - return false; - } - } - - const auto pos = input; - U result = 0; - U temp = 0; - - for (; *input && std::isdigit(*input); ++input) - { - temp = result * 10 + (*input - '0'); - - if (max_value - temp > max_value - result) - { - return false; // Overflow protection - } - - result = temp; - } - - result *= (is_negative) ? -1 : 1; - - // Reject if nothing parsed - if (pos != input) - { - output = result; - return true; - } - - return false; -} - -template ::value, bool> = true> -constexpr bool ParseNumber(T &&input, U &output) -{ - // Skip spaces - for (; *input && std::isspace(*input); ++input) - ; - - const auto is_negative = (*input && *input == '-'); - - if (is_negative) - { - ++input; - } - - const auto pos = input; - U result = 0; - U temp = 0; - U decimal_mul = 0.1f; - bool is_decimal = false; - - for (; *input && (std::isdigit(*input) || !is_decimal); ++input) - { - if (*input == '.' && !is_decimal) - { - is_decimal = true; - continue; - } - else if (*input == '.' && is_decimal) - { - break; - } - else if (is_decimal) - { - temp = result + decimal_mul * (*input - '0'); - decimal_mul *= 0.1f; - } - else - { - temp = result * 10 + (*input - '0'); - } - - if (std::isinf(temp)) - { - return false; // Overflow protection - } - - result = temp; - } - - result *= (is_negative) ? -1.0f : 1.0f; - - // Reject if nothing parsed - if (pos != input && !std::isnan(output) && !std::isinf(output)) - { - output = result; - return true; - } - - return false; -} - bool GetRawEnvironmentVariable(const char *env_var_name, std::string &value) { #if !defined(NO_GETENV) @@ -203,7 +91,16 @@ static bool GetTimeoutFromString(const char *input, std::chrono::system_clock::d { std::chrono::system_clock::duration::rep result = 0; - if (!ParseNumber(input, result)) + // Skip spaces + for (; *input && std::isspace(*input); ++input) + ; + + for (; *input && std::isdigit(*input); ++input) + { + result = result * 10 + (*input - '0'); + } + + if (result == 0) { // Rejecting duration 0 as invalid. return false; @@ -304,22 +201,38 @@ bool GetUintEnvironmentVariable(const char *env_var_name, std::uint32_t &value) static constexpr auto kDefaultValue = 0U; std::string raw_value; bool exists = GetRawEnvironmentVariable(env_var_name, raw_value); + if (!exists || raw_value.empty()) { value = kDefaultValue; return false; } - if (!ParseNumber(raw_value.c_str(), value)) + const char *end = raw_value.c_str() + raw_value.length(); + char *actual_end = nullptr; + const auto temp = std::strtoull(raw_value.c_str(), &actual_end, 10); + + if (errno == ERANGE) + { + errno = 0; + OTEL_INTERNAL_LOG_WARN("Environment variable <" << env_var_name << "> is out of range <" + << raw_value << ">, defaulting to " + << kDefaultValue); + } + else if (actual_end != end || std::numeric_limits::max() < temp) { OTEL_INTERNAL_LOG_WARN("Environment variable <" << env_var_name << "> has an invalid value <" << raw_value << ">, defaulting to " << kDefaultValue); - value = kDefaultValue; - return false; + } + else + { + value = static_cast(temp); + return true; } - return true; + value = kDefaultValue; + return false; } bool GetFloatEnvironmentVariable(const char *env_var_name, float &value) @@ -327,22 +240,37 @@ bool GetFloatEnvironmentVariable(const char *env_var_name, float &value) static constexpr auto kDefaultValue = 0.0f; std::string raw_value; bool exists = GetRawEnvironmentVariable(env_var_name, raw_value); + if (!exists || raw_value.empty()) { value = kDefaultValue; return false; } - if (!ParseNumber(raw_value.c_str(), value)) + const char *end = raw_value.c_str() + raw_value.length(); + char *actual_end = nullptr; + value = std::strtof(raw_value.c_str(), &actual_end); + + if (errno == ERANGE) + { + errno = 0; + OTEL_INTERNAL_LOG_WARN("Environment variable <" << env_var_name << "> is out of range <" + << raw_value << ">, defaulting to " + << kDefaultValue); + } + else if (actual_end != end) { OTEL_INTERNAL_LOG_WARN("Environment variable <" << env_var_name << "> has an invalid value <" << raw_value << ">, defaulting to " << kDefaultValue); - value = kDefaultValue; - return false; + } + else + { + return true; } - return true; + value = kDefaultValue; + return false; } } // namespace common diff --git a/sdk/test/common/env_var_test.cc b/sdk/test/common/env_var_test.cc index d529f3fcf9..b4cc0d2e12 100644 --- a/sdk/test/common/env_var_test.cc +++ b/sdk/test/common/env_var_test.cc @@ -2,8 +2,10 @@ // SPDX-License-Identifier: Apache-2.0 #include -#include + #include +#include +#include #include #include "opentelemetry/sdk/common/env_variables.h" @@ -224,6 +226,8 @@ TEST(EnvVarTest, UintEnvVar) setenv("UINT_ENV_VAR_POSITIVE_OVERFLOW", "4294967296", 1); setenv("UINT_ENV_VAR_NEGATIVE_INT_MIN", "-2147483648", 1); setenv("UINT_ENV_VAR_NEGATIVE_OVERFLOW", "-4294967296", 1); + setenv("UINT_ENV_VAR_TOO_LARGE_INT", "99999999999999999999", 1); + setenv("UINT_ENV_VAR_TOO_LARGE_DEC", "3.9999e+99", 1); setenv("UINT_ENV_VAR_WITH_NOISE", " \t \n 9.12345678.9", 1); setenv("UINT_ENV_VAR_ONLY_SPACES", " ", 1); @@ -238,8 +242,7 @@ TEST(EnvVarTest, UintEnvVar) ASSERT_FALSE(GetUintEnvironmentVariable("UINT_ENV_VAR_NEGATIVE_INT", value)); - ASSERT_TRUE(GetUintEnvironmentVariable("UINT_ENV_VAR_POSITIVE_DEC", value)); - ASSERT_EQ(12, value); + ASSERT_FALSE(GetUintEnvironmentVariable("UINT_ENV_VAR_POSITIVE_DEC", value)); ASSERT_FALSE(GetUintEnvironmentVariable("UINT_ENV_VAR_NEGATIVE_DEC", value)); @@ -252,8 +255,11 @@ TEST(EnvVarTest, UintEnvVar) ASSERT_FALSE(GetUintEnvironmentVariable("UINT_ENV_VAR_NEGATIVE_OVERFLOW", value)); - ASSERT_TRUE(GetUintEnvironmentVariable("UINT_ENV_VAR_WITH_NOISE", value)); - ASSERT_EQ(9, value); + ASSERT_FALSE(GetUintEnvironmentVariable("UINT_ENV_VAR_TOO_LARGE_INT", value)); + + ASSERT_FALSE(GetUintEnvironmentVariable("UINT_ENV_VAR_TOO_LARGE_DEC", value)); + + ASSERT_FALSE(GetUintEnvironmentVariable("UINT_ENV_VAR_WITH_NOISE", value)); ASSERT_FALSE(GetUintEnvironmentVariable("UINT_ENV_VAR_ONLY_SPACES", value)); @@ -266,6 +272,8 @@ TEST(EnvVarTest, UintEnvVar) unsetenv("UINT_ENV_VAR_POSITIVE_OVERFLOW"); unsetenv("UINT_ENV_VAR_NEGATIVE_INT_MIN"); unsetenv("UINT_ENV_VAR_NEGATIVE_OVERFLOW"); + unsetenv("UINT_ENV_VAR_TOO_LARGE_INT"); + unsetenv("UINT_ENV_VAR_TOO_LARGE_DEC"); unsetenv("UINT_ENV_VAR_WITH_NOISE"); unsetenv("UINT_ENV_VAR_ONLY_SPACES"); } @@ -282,6 +290,8 @@ TEST(EnvVarTest, FloatEnvVar) setenv("FLOAT_ENV_VAR_POSITIVE_OVERFLOW", "4294967296", 1); setenv("FLOAT_ENV_VAR_NEGATIVE_INT_MIN", "-2147483648", 1); setenv("FLOAT_ENV_VAR_NEGATIVE_OVERFLOW", "-4294967296", 1); + setenv("FLOAT_ENV_VAR_TOO_LARGE_INT", "99999999999999999999", 1); + setenv("FLOAT_ENV_VAR_TOO_LARGE_DEC", "3.9999e+99", 1); setenv("FLOAT_ENV_VAR_WITH_NOISE", " \t \n 9.12345678.9", 1); setenv("FLOAT_ENV_VAR_ONLY_SPACES", " ", 1); @@ -315,8 +325,12 @@ TEST(EnvVarTest, FloatEnvVar) ASSERT_TRUE(GetFloatEnvironmentVariable("FLOAT_ENV_VAR_NEGATIVE_OVERFLOW", value)); ASSERT_FLOAT_EQ(-4294967296.f, value); - ASSERT_TRUE(GetFloatEnvironmentVariable("FLOAT_ENV_VAR_WITH_NOISE", value)); - ASSERT_FLOAT_EQ(9.12345678f, value); + ASSERT_TRUE(GetFloatEnvironmentVariable("FLOAT_ENV_VAR_TOO_LARGE_INT", value)); + ASSERT_FLOAT_EQ(99999999999999999999.f, value); + + ASSERT_FALSE(GetFloatEnvironmentVariable("FLOAT_ENV_VAR_TOO_LARGE_DEC", value)); + + ASSERT_FALSE(GetFloatEnvironmentVariable("FLOAT_ENV_VAR_WITH_NOISE", value)); ASSERT_FALSE(GetFloatEnvironmentVariable("FLOAT_ENV_VAR_ONLY_SPACES", value)); @@ -329,6 +343,8 @@ TEST(EnvVarTest, FloatEnvVar) unsetenv("FLOAT_ENV_VAR_POSITIVE_OVERFLOW"); unsetenv("FLOAT_ENV_VAR_NEGATIVE_INT_MIN"); unsetenv("FLOAT_ENV_VAR_NEGATIVE_OVERFLOW"); + unsetenv("FLOAT_ENV_VAR_TOO_LARGE_INT"); + unsetenv("FLOAT_ENV_VAR_TOO_LARGE_DEC"); unsetenv("FLOAT_ENV_VAR_WITH_NOISE"); unsetenv("FLOAT_ENV_VAR_ONLY_SPACES"); } From 9349d6c912d49dcec19d7dcf98b91ca2c16b0665 Mon Sep 17 00:00:00 2001 From: Alex E Date: Thu, 9 Jan 2025 22:31:57 -0500 Subject: [PATCH 12/16] Fix remaining issues after merge --- CHANGELOG.md | 3 ++ exporters/otlp/src/otlp_grpc_client.cc | 13 +++-- .../otlp/test/otlp_grpc_exporter_test.cc | 54 ++++++++++--------- .../otlp_grpc_log_record_exporter_test.cc | 44 +++++++-------- .../test/otlp_grpc_metric_exporter_test.cc | 44 +++++++-------- 5 files changed, 84 insertions(+), 74 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52f280eb22..ec19d28f56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,9 @@ Increment the: * [EXPORTER] Fix scope attributes missing from otlp traces metrics [#3185](https://github.com/open-telemetry/opentelemetry-cpp/pull/3185) + [EXPORTER] Support handling retry-able errors for OTLP/gRPC + [#3219](https://github.com/open-telemetry/opentelemetry-cpp/pull/3219) + ## [1.18 2024-11-25] * [EXPORTER] Fix crash in ElasticsearchLogRecordExporter diff --git a/exporters/otlp/src/otlp_grpc_client.cc b/exporters/otlp/src/otlp_grpc_client.cc index 7b73082cb7..5b97703bdd 100644 --- a/exporters/otlp/src/otlp_grpc_client.cc +++ b/exporters/otlp/src/otlp_grpc_client.cc @@ -349,8 +349,11 @@ std::shared_ptr OtlpGrpcClient::MakeChannel(const OtlpGrpcClientO grpc_arguments.SetCompressionAlgorithm(GRPC_COMPRESS_GZIP); } - if (options.retry_policy_max_attempts > 0U && options.retry_policy_initial_backoff > 0.0f && - options.retry_policy_max_backoff > 0.0f && options.retry_policy_backoff_multiplier > 0.0f) +#ifdef ENABLE_OTLP_RETRY_PREVIEW + if (options.retry_policy_max_attempts > 0U && + options.retry_policy_initial_backoff > std::chrono::duration::zero() && + options.retry_policy_max_backoff > std::chrono::duration::zero() && + options.retry_policy_backoff_multiplier > 0.0f) { static const auto kServiceConfigJson = opentelemetry::nostd::string_view{R"( { @@ -380,11 +383,13 @@ std::shared_ptr OtlpGrpcClient::MakeChannel(const OtlpGrpcClientO // Prior to C++17, need to explicitly cast away constness from `data()` buffer std::snprintf(const_cast(service_config.data()), service_config.size(), kServiceConfigJson.data(), - options.retry_policy_max_attempts, options.retry_policy_initial_backoff, - options.retry_policy_max_backoff, options.retry_policy_backoff_multiplier); + options.retry_policy_max_attempts, options.retry_policy_initial_backoff.count(), + options.retry_policy_max_backoff.count(), + options.retry_policy_backoff_multiplier); grpc_arguments.SetServiceConfigJSON(service_config); } +#endif // ENABLE_OTLP_RETRY_PREVIEW if (options.use_ssl_credentials) { diff --git a/exporters/otlp/test/otlp_grpc_exporter_test.cc b/exporters/otlp/test/otlp_grpc_exporter_test.cc index be1ddbb3cf..5c4040b350 100644 --- a/exporters/otlp/test/otlp_grpc_exporter_test.cc +++ b/exporters/otlp/test/otlp_grpc_exporter_test.cc @@ -359,52 +359,53 @@ TEST_F(OtlpGrpcExporterTestPeer, ConfigRetryDefaultValues) std::unique_ptr exporter(new OtlpGrpcExporter()); const auto options = GetOptions(exporter); ASSERT_EQ(options.retry_policy_max_attempts, 5); - ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff, 1.0); - ASSERT_FLOAT_EQ(options.retry_policy_max_backoff, 5.0); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 1.0); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 5.0); ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 1.5); } TEST_F(OtlpGrpcExporterTestPeer, ConfigRetryValuesFromEnv) { - setenv("OTEL_EXPORTER_OTLP_TRACES_RETRY_MAX_ATTEMPTS", "123", 1); - setenv("OTEL_EXPORTER_OTLP_TRACES_RETRY_INITIAL_BACKOFF", "4.5", 1); - setenv("OTEL_EXPORTER_OTLP_TRACES_RETRY_MAX_BACKOFF", "6.7", 1); - setenv("OTEL_EXPORTER_OTLP_TRACES_RETRY_BACKOFF_MULTIPLIER", "8.9", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_TRACES_RETRY_MAX_ATTEMPTS", "123", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_TRACES_RETRY_INITIAL_BACKOFF", "4.5", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_TRACES_RETRY_MAX_BACKOFF", "6.7", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_TRACES_RETRY_BACKOFF_MULTIPLIER", "8.9", 1); std::unique_ptr exporter(new OtlpGrpcExporter()); const auto options = GetOptions(exporter); ASSERT_EQ(options.retry_policy_max_attempts, 123); - ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff, 4.5); - ASSERT_FLOAT_EQ(options.retry_policy_max_backoff, 6.7); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 4.5); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 6.7); ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 8.9); - unsetenv("OTEL_EXPORTER_OTLP_TRACES_RETRY_MAX_ATTEMPTS"); - unsetenv("OTEL_EXPORTER_OTLP_TRACES_RETRY_INITIAL_BACKOFF"); - unsetenv("OTEL_EXPORTER_OTLP_TRACES_RETRY_MAX_BACKOFF"); - unsetenv("OTEL_EXPORTER_OTLP_TRACES_RETRY_BACKOFF_MULTIPLIER"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_TRACES_RETRY_MAX_ATTEMPTS"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_TRACES_RETRY_INITIAL_BACKOFF"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_TRACES_RETRY_MAX_BACKOFF"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_TRACES_RETRY_BACKOFF_MULTIPLIER"); } TEST_F(OtlpGrpcExporterTestPeer, ConfigRetryGenericValuesFromEnv) { - setenv("OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS", "321", 1); - setenv("OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF", "5.4", 1); - setenv("OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF", "7.6", 1); - setenv("OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER", "9.8", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS", "321", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF", "5.4", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_RETRY_MAX_BACKOFF", "7.6", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER", "9.8", 1); std::unique_ptr exporter(new OtlpGrpcExporter()); const auto options = GetOptions(exporter); ASSERT_EQ(options.retry_policy_max_attempts, 321); - ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff, 5.4); - ASSERT_FLOAT_EQ(options.retry_policy_max_backoff, 7.6); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 5.4); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 7.6); ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 9.8); - unsetenv("OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"); - unsetenv("OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"); - unsetenv("OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"); - unsetenv("OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_RETRY_MAX_BACKOFF"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"); } # endif // NO_GETENV +# ifdef ENABLE_OTLP_RETRY_PREVIEW struct TestTraceService : public opentelemetry::proto::collector::trace::v1::TraceService::Service { TestTraceService(std::vector status_codes) : status_codes_(status_codes) {} @@ -523,15 +524,15 @@ TEST_P(OtlpGrpcExporterRetryIntegrationTests, StatusCodes) if (is_retry_enabled) { opts.retry_policy_max_attempts = 5; - opts.retry_policy_initial_backoff = 0.1f; - opts.retry_policy_max_backoff = 5.0f; + opts.retry_policy_initial_backoff = std::chrono::duration{0.1f}; + opts.retry_policy_max_backoff = std::chrono::duration{5.0f}; opts.retry_policy_backoff_multiplier = 1.0f; } else { opts.retry_policy_max_attempts = 0; - opts.retry_policy_initial_backoff = 0.0f; - opts.retry_policy_max_backoff = 0.0f; + opts.retry_policy_initial_backoff = std::chrono::duration::zero(); + opts.retry_policy_max_backoff = std::chrono::duration::zero(); opts.retry_policy_backoff_multiplier = 0.0f; } @@ -551,6 +552,7 @@ TEST_P(OtlpGrpcExporterRetryIntegrationTests, StatusCodes) ASSERT_EQ(expected_attempts, service.request_count_); } +# endif // ENABLE_OTLP_RETRY_PREVIEW } // namespace otlp } // namespace exporter diff --git a/exporters/otlp/test/otlp_grpc_log_record_exporter_test.cc b/exporters/otlp/test/otlp_grpc_log_record_exporter_test.cc index 1dc9fd5b74..880f2f89b4 100644 --- a/exporters/otlp/test/otlp_grpc_log_record_exporter_test.cc +++ b/exporters/otlp/test/otlp_grpc_log_record_exporter_test.cc @@ -472,49 +472,49 @@ TEST_F(OtlpGrpcLogRecordExporterTestPeer, ConfigRetryDefaultValues) std::unique_ptr exporter(new OtlpGrpcLogRecordExporter()); const auto options = GetOptions(exporter); ASSERT_EQ(options.retry_policy_max_attempts, 5); - ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff, 1.0); - ASSERT_FLOAT_EQ(options.retry_policy_max_backoff, 5.0); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 1.0); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 5.0); ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 1.5); } TEST_F(OtlpGrpcLogRecordExporterTestPeer, ConfigRetryValuesFromEnv) { - setenv("OTEL_EXPORTER_OTLP_LOGS_RETRY_MAX_ATTEMPTS", "123", 1); - setenv("OTEL_EXPORTER_OTLP_LOGS_RETRY_INITIAL_BACKOFF", "4.5", 1); - setenv("OTEL_EXPORTER_OTLP_LOGS_RETRY_MAX_BACKOFF", "6.7", 1); - setenv("OTEL_EXPORTER_OTLP_LOGS_RETRY_BACKOFF_MULTIPLIER", "8.9", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_LOGS_RETRY_MAX_ATTEMPTS", "123", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_LOGS_RETRY_INITIAL_BACKOFF", "4.5", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_LOGS_RETRY_MAX_BACKOFF", "6.7", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_LOGS_RETRY_BACKOFF_MULTIPLIER", "8.9", 1); std::unique_ptr exporter(new OtlpGrpcLogRecordExporter()); const auto options = GetOptions(exporter); ASSERT_EQ(options.retry_policy_max_attempts, 123); - ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff, 4.5); - ASSERT_FLOAT_EQ(options.retry_policy_max_backoff, 6.7); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 4.5); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 6.7); ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 8.9); - unsetenv("OTEL_EXPORTER_OTLP_LOGS_RETRY_MAX_ATTEMPTS"); - unsetenv("OTEL_EXPORTER_OTLP_LOGS_RETRY_INITIAL_BACKOFF"); - unsetenv("OTEL_EXPORTER_OTLP_LOGS_RETRY_MAX_BACKOFF"); - unsetenv("OTEL_EXPORTER_OTLP_LOGS_RETRY_BACKOFF_MULTIPLIER"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_LOGS_RETRY_MAX_ATTEMPTS"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_LOGS_RETRY_INITIAL_BACKOFF"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_LOGS_RETRY_MAX_BACKOFF"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_LOGS_RETRY_BACKOFF_MULTIPLIER"); } TEST_F(OtlpGrpcLogRecordExporterTestPeer, ConfigRetryGenericValuesFromEnv) { - setenv("OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS", "321", 1); - setenv("OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF", "5.4", 1); - setenv("OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF", "7.6", 1); - setenv("OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER", "9.8", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS", "321", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF", "5.4", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_RETRY_MAX_BACKOFF", "7.6", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER", "9.8", 1); std::unique_ptr exporter(new OtlpGrpcLogRecordExporter()); const auto options = GetOptions(exporter); ASSERT_EQ(options.retry_policy_max_attempts, 321); - ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff, 5.4); - ASSERT_FLOAT_EQ(options.retry_policy_max_backoff, 7.6); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 5.4); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 7.6); ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 9.8); - unsetenv("OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"); - unsetenv("OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"); - unsetenv("OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"); - unsetenv("OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_RETRY_MAX_BACKOFF"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"); } #endif // NO_GETENV diff --git a/exporters/otlp/test/otlp_grpc_metric_exporter_test.cc b/exporters/otlp/test/otlp_grpc_metric_exporter_test.cc index 661b4808b9..5d39cff4d9 100644 --- a/exporters/otlp/test/otlp_grpc_metric_exporter_test.cc +++ b/exporters/otlp/test/otlp_grpc_metric_exporter_test.cc @@ -200,49 +200,49 @@ TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigRetryDefaultValues) std::unique_ptr exporter(new OtlpGrpcMetricExporter()); const auto options = GetOptions(exporter); ASSERT_EQ(options.retry_policy_max_attempts, 5); - ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff, 1.0); - ASSERT_FLOAT_EQ(options.retry_policy_max_backoff, 5.0); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 1.0); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 5.0); ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 1.5); } TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigRetryValuesFromEnv) { - setenv("OTEL_EXPORTER_OTLP_METRICS_RETRY_MAX_ATTEMPTS", "123", 1); - setenv("OTEL_EXPORTER_OTLP_METRICS_RETRY_INITIAL_BACKOFF", "4.5", 1); - setenv("OTEL_EXPORTER_OTLP_METRICS_RETRY_MAX_BACKOFF", "6.7", 1); - setenv("OTEL_EXPORTER_OTLP_METRICS_RETRY_BACKOFF_MULTIPLIER", "8.9", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_METRICS_RETRY_MAX_ATTEMPTS", "123", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_METRICS_RETRY_INITIAL_BACKOFF", "4.5", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_METRICS_RETRY_MAX_BACKOFF", "6.7", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_METRICS_RETRY_BACKOFF_MULTIPLIER", "8.9", 1); std::unique_ptr exporter(new OtlpGrpcMetricExporter()); const auto options = GetOptions(exporter); ASSERT_EQ(options.retry_policy_max_attempts, 123); - ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff, 4.5); - ASSERT_FLOAT_EQ(options.retry_policy_max_backoff, 6.7); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 4.5); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 6.7); ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 8.9); - unsetenv("OTEL_EXPORTER_OTLP_METRICS_RETRY_MAX_ATTEMPTS"); - unsetenv("OTEL_EXPORTER_OTLP_METRICS_RETRY_INITIAL_BACKOFF"); - unsetenv("OTEL_EXPORTER_OTLP_METRICS_RETRY_MAX_BACKOFF"); - unsetenv("OTEL_EXPORTER_OTLP_METRICS_RETRY_BACKOFF_MULTIPLIER"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_METRICS_RETRY_MAX_ATTEMPTS"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_METRICS_RETRY_INITIAL_BACKOFF"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_METRICS_RETRY_MAX_BACKOFF"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_METRICS_RETRY_BACKOFF_MULTIPLIER"); } TEST_F(OtlpGrpcMetricExporterTestPeer, ConfigRetryGenericValuesFromEnv) { - setenv("OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS", "321", 1); - setenv("OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF", "5.4", 1); - setenv("OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF", "7.6", 1); - setenv("OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER", "9.8", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS", "321", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF", "5.4", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_RETRY_MAX_BACKOFF", "7.6", 1); + setenv("OTEL_CPP_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER", "9.8", 1); std::unique_ptr exporter(new OtlpGrpcMetricExporter()); const auto options = GetOptions(exporter); ASSERT_EQ(options.retry_policy_max_attempts, 321); - ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff, 5.4); - ASSERT_FLOAT_EQ(options.retry_policy_max_backoff, 7.6); + ASSERT_FLOAT_EQ(options.retry_policy_initial_backoff.count(), 5.4); + ASSERT_FLOAT_EQ(options.retry_policy_max_backoff.count(), 7.6); ASSERT_FLOAT_EQ(options.retry_policy_backoff_multiplier, 9.8); - unsetenv("OTEL_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"); - unsetenv("OTEL_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"); - unsetenv("OTEL_EXPORTER_OTLP_RETRY_MAX_BACKOFF"); - unsetenv("OTEL_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_RETRY_MAX_ATTEMPTS"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_RETRY_INITIAL_BACKOFF"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_RETRY_MAX_BACKOFF"); + unsetenv("OTEL_CPP_EXPORTER_OTLP_RETRY_BACKOFF_MULTIPLIER"); } # endif // NO_GETENV From d711cf098eddfec1593da8edcee594f58dc8a0e3 Mon Sep 17 00:00:00 2001 From: Alex E Date: Fri, 10 Jan 2025 07:14:20 -0500 Subject: [PATCH 13/16] Enable unit testing gRPC retries in CI --- ci/do_ci.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/do_ci.sh b/ci/do_ci.sh index 0f6add0ff5..5a75069ee5 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -360,6 +360,7 @@ elif [[ "$1" == "cmake.exporter.otprotocol.test" ]]; then -DWITH_OTLP_HTTP=ON \ -DWITH_OTLP_FILE=ON \ -DWITH_OTLP_GRPC_SSL_MTLS_PREVIEW=ON \ + -DWITH_OTLP_RETRY_PREVIEW=ON \ "${SRC_DIR}" grpc_cpp_plugin=`which grpc_cpp_plugin` proto_make_file="CMakeFiles/opentelemetry_proto.dir/build.make" From 12cc3b79015eddd26e1ac3aaaec00618877341f0 Mon Sep 17 00:00:00 2001 From: Alex E Date: Sun, 12 Jan 2025 17:28:12 -0500 Subject: [PATCH 14/16] Guard against large float values --- exporters/otlp/src/otlp_grpc_client.cc | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/exporters/otlp/src/otlp_grpc_client.cc b/exporters/otlp/src/otlp_grpc_client.cc index 5b97703bdd..4d2fb6d38e 100644 --- a/exporters/otlp/src/otlp_grpc_client.cc +++ b/exporters/otlp/src/otlp_grpc_client.cc @@ -362,9 +362,9 @@ std::shared_ptr OtlpGrpcClient::MakeChannel(const OtlpGrpcClientO "name": [{}], "retryPolicy": { "maxAttempts": %0000000000u, - "initialBackoff": "%.1fs", - "maxBackoff": "%.1fs", - "backoffMultiplier": %.1f, + "initialBackoff": "%0000000000.1fs", + "maxBackoff": "%0000000000.1fs", + "backoffMultiplier": %0000000000.1f, "retryableStatusCodes": [ "CANCELLED", "DEADLINE_EXCEEDED", @@ -381,11 +381,12 @@ std::shared_ptr OtlpGrpcClient::MakeChannel(const OtlpGrpcClientO // Allocate string with buffer large enough to hold the formatted json config auto service_config = std::string(kServiceConfigJson.size(), '\0'); // Prior to C++17, need to explicitly cast away constness from `data()` buffer - std::snprintf(const_cast(service_config.data()), - service_config.size(), kServiceConfigJson.data(), - options.retry_policy_max_attempts, options.retry_policy_initial_backoff.count(), - options.retry_policy_max_backoff.count(), - options.retry_policy_backoff_multiplier); + std::snprintf( + const_cast(service_config.data()), + service_config.size(), kServiceConfigJson.data(), options.retry_policy_max_attempts, + std::min(std::max(options.retry_policy_initial_backoff.count(), 0.f), 999999999.f), + std::min(std::max(options.retry_policy_max_backoff.count(), 0.f), 999999999.f), + std::min(std::max(options.retry_policy_backoff_multiplier, 0.f), 999999999.f)); grpc_arguments.SetServiceConfigJSON(service_config); } From 33fcb79ee01b09c0f1dde1a6ff1fe8a0028f3430 Mon Sep 17 00:00:00 2001 From: Alex E Date: Fri, 17 Jan 2025 12:16:35 -0500 Subject: [PATCH 15/16] Fix markdown --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d597249f9..eb5675bed6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,7 +96,6 @@ New features: as well as the thread instrumentation interface, may change without notice before this feature is declared stable. - ## [1.18 2024-11-25] * [EXPORTER] Fix crash in ElasticsearchLogRecordExporter From b537a494548d8f67479affaa5ab980f41a5cbe78 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Tue, 21 Jan 2025 17:29:25 +0100 Subject: [PATCH 16/16] Update exporters/otlp/test/otlp_grpc_exporter_test.cc --- exporters/otlp/test/otlp_grpc_exporter_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/exporters/otlp/test/otlp_grpc_exporter_test.cc b/exporters/otlp/test/otlp_grpc_exporter_test.cc index 5a405c2824..c3ba455c09 100644 --- a/exporters/otlp/test/otlp_grpc_exporter_test.cc +++ b/exporters/otlp/test/otlp_grpc_exporter_test.cc @@ -411,9 +411,9 @@ struct TestTraceService : public opentelemetry::proto::collector::trace::v1::Tra TestTraceService(std::vector status_codes) : status_codes_(status_codes) {} inline grpc::Status Export( - grpc::ServerContext *context, - const opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest *request, - opentelemetry::proto::collector::trace::v1::ExportTraceServiceResponse *response) override + grpc::ServerContext *, + const opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest *, + opentelemetry::proto::collector::trace::v1::ExportTraceServiceResponse *) override { ++request_count_; return grpc::Status(status_codes_.at(index_++ % status_codes_.size()), "TEST!");