Make Otlp exporter configuration environment variables specs-compliant (#974)

lalitb · ThomsonTan · web-flow · commit 43d8fe42dd3b · 2021-09-14T14:28:07.000-07:00
* add documentation for GRPC Env variables

* Update exporters/otlp/README.md

Co-authored-by: Tom Tan &lt;lilotom@gmail.com&gt;

* Update exporters/otlp/README.md

Co-authored-by: Tom Tan &lt;lilotom@gmail.com&gt;

* spec compliant env var

* markdown lint

Co-authored-by: Tom Tan &lt;lilotom@gmail.com&gt;
diff --git a/exporters/otlp/README.md b/exporters/otlp/README.md
@@ -42,11 +42,28 @@ options.url = "localhost:12345";
 auto exporter = std::unique_ptr<sdktrace::SpanExporter>(new otlp::OtlpHttpExporter(options));
 ```
 
-### Configuration options
+### Configuration options ( OTLP GRPC Exporter )
 
-| Option       | Default          |
-| ------------ |----------------- |
-| `endpoint`   | `localhost:4317` |
+| Option       | Env Variable |Default          |   Description  |
+| ------------ |---------------|------------ |----------------|
+| `endpoint` | `OTEL_EXPORTER_OTLP_ENDPOINT` | `http://localhost:4317`| The OTLP GRPC endpoint to connect to |
+|              |   `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT`  |  | |
+| `use_ssl_credentials` | `OTEL_EXPORTER_OTLP_SSL_ENABLE`| `false` | Whether the endpoint is SSL enabled |
+|    | `OTEL_EXPORTER_OTLP_TRACES_SSL_ENABLE` | | |
+| `ssl_credentials_cacert_path`  |  `OTEL_EXPORTER_OTLP_CERTIFICATE` | `""`  | SSL Certificate file path |
+|    | `OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE` | | |
+| `ssl_credentials_cacert_as_string` | `OTEL_EXPORTER_OTLP_CERTIFICATE_STRING` | `""`  |   SSL Certifcate as in-memory string |
+|  | `OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE_STRING` | | | |
+
+### Configuration options ( OTLP HTTP Exporter )
+
+| Option       | Env Variable |Default          | Description |
+| ------------ |-----|------------ |------|
+| `url`   | n/a    | `http://localhost:4317/v1/traces`  | The OTLP HTTP endpoint to connect to |
+| `content_type` | n/a  | `application/json`  |   Data format used - JSON or Binary |
+| `json_bytes_mapping`  |  n/a | `JsonBytesMappingKind::kHexId` | Encoding used for trace_id and span_id |
+| `use_json_name` | n/a | `false`  | Whether to use json name of protobuf field to set the key of json |
+| `timeout`  | n/a  | `30000 ms` | http timeout |
 
 ## Example
 
diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_exporter.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_exporter.h
@@ -19,30 +19,60 @@ namespace exporter
 namespace otlp
 {
 
-inline const std::string GetOtlpGrpcDefaultEndpoint()
+inline const std::string GetOtlpDefaultEndpoint()
 {
-  constexpr char kOtlpGrpcEndpointEnv[]     = "OTEL_EXPORTER_OTLP_GRPC_ENDPOINT";
-  constexpr char kOtlpGrpcEndpointDefault[] = "localhost:4317";
+  constexpr char kOtlpTracesEndpointEnv[] = "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT";
+  constexpr char kOtlpEndpointEnv[]       = "OTEL_EXPORTER_OTLP_ENDPOINT";
+  constexpr char kOtlpEndpointDefault[]   = "http://localhost:4317";
 
-  auto endpoint = opentelemetry::sdk::common::GetEnvironmentVariable(kOtlpGrpcEndpointEnv);
-  return endpoint.size() ? endpoint : kOtlpGrpcEndpointDefault;
+  auto endpoint = opentelemetry::sdk::common::GetEnvironmentVariable(kOtlpTracesEndpointEnv);
+  if (endpoint.size() == 0)
+  {
+    endpoint = opentelemetry::sdk::common::GetEnvironmentVariable(kOtlpEndpointEnv);
+  }
+  return endpoint.size() ? endpoint : kOtlpEndpointDefault;
 }
 
-inline const bool GetOtlpGrpcDefaultIsSslEnable()
+inline const bool GetOtlpDefaultIsSslEnable()
 {
-  constexpr char kOtlpGrpcIsSslEnableEnv[] = "OTEL_EXPORTER_OTLP_GRPC_SSL_ENABLE";
-  auto ssl_enable = opentelemetry::sdk::common::GetEnvironmentVariable(kOtlpGrpcIsSslEnableEnv);
+  constexpr char kOtlpTracesIsSslEnableEnv[] = "OTEL_EXPORTER_OTLP_TRACES_SSL_ENABLE";
+  constexpr char kOtlpIsSslEnableEnv[]       = "OTEL_EXPORTER_OTLP_SSL_ENABLE";
+
+  auto ssl_enable = opentelemetry::sdk::common::GetEnvironmentVariable(kOtlpTracesIsSslEnableEnv);
+  if (ssl_enable.size() == 0)
+  {
+    ssl_enable = opentelemetry::sdk::common::GetEnvironmentVariable(kOtlpIsSslEnableEnv);
+  }
   if (ssl_enable == "True" || ssl_enable == "TRUE" || ssl_enable == "true" || ssl_enable == "1")
   {
     return true;
   }
   return false;
 }
 
-inline const std::string GetOtlpGrpcDefaultSslCertificate()
+inline const std::string GetOtlpDefaultSslCertificatePath()
+{
+  constexpr char kOtlpTracesSslCertificate[] = "OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE";
+  constexpr char kOtlpSslCertificate[]       = "OTEL_EXPORTER_OTLP_CERTIFICATE ";
+  auto ssl_cert_path =
+      opentelemetry::sdk::common::GetEnvironmentVariable(kOtlpTracesSslCertificate);
+  if (ssl_cert_path.size() == 0)
+  {
+    ssl_cert_path = opentelemetry::sdk::common::GetEnvironmentVariable(kOtlpSslCertificate);
+  }
+  return ssl_cert_path.size() ? ssl_cert_path : "";
+}
+
+inline const std::string GetOtlpDefaultSslCertificateString()
 {
-  constexpr char kOtlpGrpcSslCertificate[] = "OTEL_EXPORTER_OTLP_GRPC_SSL_CERTIFICATE";
-  auto ssl_cert = opentelemetry::sdk::common::GetEnvironmentVariable(kOtlpGrpcSslCertificate);
+  constexpr char kOtlpTracesSslCertificateString[] = "OTEL_EXPORTER_OTLP_CERTIFICATE_STRING";
+  constexpr char kOtlpSslCertificateString[] = "OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE_STRING ";
+  auto ssl_cert =
+      opentelemetry::sdk::common::GetEnvironmentVariable(kOtlpTracesSslCertificateString);
+  if (ssl_cert.size() == 0)
+  {
+    ssl_cert = opentelemetry::sdk::common::GetEnvironmentVariable(kOtlpSslCertificateString);
+  }
   return ssl_cert.size() ? ssl_cert : "";
 }
 
@@ -52,15 +82,15 @@ inline const std::string GetOtlpGrpcDefaultSslCertificate()
 struct OtlpGrpcExporterOptions
 {
   // The endpoint to export to. By default the OpenTelemetry Collector's default endpoint.
-  std::string endpoint = GetOtlpGrpcDefaultEndpoint();
+  std::string endpoint = GetOtlpDefaultEndpoint();
   // By default when false, uses grpc::InsecureChannelCredentials(); If true,
   // uses ssl_credentials_cacert_path if non-empty, else uses ssl_credentials_cacert_as_string
-  bool use_ssl_credentials = GetOtlpGrpcDefaultIsSslEnable();
+  bool use_ssl_credentials = GetOtlpDefaultIsSslEnable();
   // ssl_credentials_cacert_path specifies path to .pem file to be used for SSL encryption.
-  std::string ssl_credentials_cacert_path = "";
+  std::string ssl_credentials_cacert_path = GetOtlpDefaultSslCertificatePath();
   // ssl_credentials_cacert_as_string in-memory string representation of .pem file to be used for
   // SSL encryption.
-  std::string ssl_credentials_cacert_as_string = GetOtlpGrpcDefaultSslCertificate();
+  std::string ssl_credentials_cacert_as_string = GetOtlpDefaultSslCertificateString();
 };
 
 /**
diff --git a/exporters/otlp/src/otlp_grpc_exporter.cc b/exporters/otlp/src/otlp_grpc_exporter.cc
@@ -59,6 +59,7 @@ std::unique_ptr<proto::collector::trace::v1::TraceService::Stub> MakeServiceStub
     const OtlpGrpcExporterOptions &options)
 {
   std::shared_ptr<grpc::Channel> channel;
+
   if (options.use_ssl_credentials)
   {
     grpc::SslCredentialsOptions ssl_opts;
diff --git a/exporters/otlp/test/otlp_grpc_exporter_test.cc b/exporters/otlp/test/otlp_grpc_exporter_test.cc
@@ -139,27 +139,27 @@ TEST_F(OtlpGrpcExporterTestPeer, ConfigSslCredentialsTest)
 TEST_F(OtlpGrpcExporterTestPeer, ConfigFromEnv)
 {
   const std::string cacert_str = "--begin and end fake cert--";
-  const std::string cacert_env = "OTEL_EXPORTER_OTLP_GRPC_SSL_CERTIFICATE=" + cacert_str;
+  const std::string cacert_env = "OTEL_EXPORTER_OTLP_CERTIFICATE_STRING=" + cacert_str;
   putenv(const_cast<char *>(cacert_env.data()));
-  char ssl_enable_env[] = "OTEL_EXPORTER_OTLP_GRPC_SSL_ENABLE=True";
+  char ssl_enable_env[] = "OTEL_EXPORTER_OTLP_SSL_ENABLE=True";
   putenv(ssl_enable_env);
   const std::string endpoint     = "http://localhost:9999";
-  const std::string endpoint_env = "OTEL_EXPORTER_OTLP_GRPC_ENDPOINT=" + endpoint;
+  const std::string endpoint_env = "OTEL_EXPORTER_OTLP_ENDPOINT=" + endpoint;
   putenv(const_cast<char *>(endpoint_env.data()));
 
   std::unique_ptr<OtlpGrpcExporter> exporter(new OtlpGrpcExporter());
   EXPECT_EQ(GetOptions(exporter).ssl_credentials_cacert_as_string, cacert_str);
   EXPECT_EQ(GetOptions(exporter).use_ssl_credentials, true);
   EXPECT_EQ(GetOptions(exporter).endpoint, endpoint);
 #    if defined(_MSC_VER)
-  putenv("OTEL_EXPORTER_OTLP_GRPC_ENDPOINT=");
-  putenv("OTEL_EXPORTER_OTLP_GRPC_SSL_CERTIFICATE=");
-  putenv("OTEL_EXPORTER_OTLP_GRPC_SSL_ENABLE=");
+  putenv("OTEL_EXPORTER_OTLP_ENDPOINT=");
+  putenv("OTEL_EXPORTER_OTLP_CERTIFICATE_STRING=");
+  putenv("OTEL_EXPORTER_OTLP_SSL_ENABLE=");
 
 #    else
-  unsetenv("OTEL_EXPORTER_OTLP_GRPC_ENDPOINT");
-  unsetenv("OTEL_EXPORTER_OTLP_GRPC_SSL_CERTIFICATE");
-  unsetenv("OTEL_EXPORTER_OTLP_GRPC_SSL_ENABLE");
+  unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT");
+  unsetenv("OTEL_EXPORTER_OTLP_CERTIFICATE_STRING");
+  unsetenv("OTEL_EXPORTER_OTLP_SSL_ENABLE");
 
 #    endif
 }

Original file line number	Diff line number	Diff line change
`@@ -59,6 +59,7 @@ std::unique_ptr<proto::collector::trace::v1::TraceService::Stub> MakeServiceStub`
`59`	`59`	`const OtlpGrpcExporterOptions &options)`
`60`	`60`	`{`
`61`	`61`	`std::shared_ptr<grpc::Channel> channel;`
	`62`	`+`
`62`	`63`	`if (options.use_ssl_credentials)`
`63`	`64`	`{`
`64`	`65`	`grpc::SslCredentialsOptions ssl_opts;`