Move resource data into OTLP 'Resource'

Signed-off-by: Itamar Kaminski <itamark@google.com>

source/extensions/access_loggers/common/BUILD

@@ -34,7 +34,6 @@ envoy_cc_library(
         "//source/common/common:assert_lib",
         "//source/common/grpc:typed_async_client_lib",
         "//source/common/protobuf:utility_lib",
-        "//source/common/runtime:runtime_features_lib",
         "@com_google_absl//absl/types:optional",
         "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
     ],

source/extensions/access_loggers/common/grpc_access_logger.h

@@ -12,7 +12,6 @@
 #include "common/common/assert.h"
 #include "common/grpc/typed_async_client.h"
 #include "common/protobuf/utility.h"
-#include "common/runtime/runtime_features.h"
 
 #include "absl/container/flat_hash_map.h"
 #include "absl/types/optional.h"

source/extensions/access_loggers/grpc/grpc_ot_access_log_impl.cc

@@ -30,17 +30,33 @@ GrpcOpenTelemetryAccessLoggerImpl::GrpcOpenTelemetryAccessLoggerImpl(
           Grpc::VersionedMethods("opentelemetry.proto.collector.logs.v1.LogsService.Export",
                                  "opentelemetry.proto.collector.logs.v1.LogsService.Export")
               .getMethodDescriptorForVersion(transport_api_version),
-          transport_api_version),
-      log_name_(log_name), local_info_(local_info) {
-  initRoot();
+          transport_api_version) {
+  initMessageRoot(log_name, local_info);
 }
 
+namespace {
+
+opentelemetry::proto::common::v1::KeyValue getStringKeyValue(const std::string& key,
+                                                             const std::string& value) {
+  opentelemetry::proto::common::v1::KeyValue keyValue;
+  keyValue.set_key(key);
+  keyValue.mutable_value()->set_string_value(value);
+  return keyValue;
+}
+
+} // namespace
+
 // See comment about the structure of repeated fields in the header file.
-void GrpcOpenTelemetryAccessLoggerImpl::initRoot() {
-  root_ = message_.add_resource_logs()->add_instrumentation_library_logs();
-  // No reason to clear and refill these every time.
-  root_->mutable_instrumentation_library()->set_name("envoy");
-  root_->mutable_instrumentation_library()->set_version("v1");
+// TODO(itamarkam): allow user configurable attributes.
+void GrpcOpenTelemetryAccessLoggerImpl::initMessageRoot(const std::string& log_name,
+                                                        const LocalInfo::LocalInfo& local_info) {
+  auto* resource_logs = message_.add_resource_logs();
+  root_ = resource_logs->add_instrumentation_library_logs();
+  auto* resource = resource_logs->mutable_resource();
+  *resource->add_attributes() = getStringKeyValue("log_name", log_name);
+  *resource->add_attributes() = getStringKeyValue("zone_name", local_info.zoneName());
+  *resource->add_attributes() = getStringKeyValue("cluster_name", local_info.clusterName());
+  *resource->add_attributes() = getStringKeyValue("node_name", local_info.nodeName());
 }
 
 void GrpcOpenTelemetryAccessLoggerImpl::addEntry(
@@ -55,12 +71,8 @@ void GrpcOpenTelemetryAccessLoggerImpl::addEntry(
 
 bool GrpcOpenTelemetryAccessLoggerImpl::isEmpty() { return root_->logs().empty(); }
 
-void GrpcOpenTelemetryAccessLoggerImpl::initMessage() {
-  // auto* resource = message_.mutable_resource_logs(0)->mutable_resource();
-  // resource->add_attributes() ..
-  (void)log_name_;
-  (void)local_info_;
-}
+// The message is already initialized in the c'tor, and only the logs are cleared.
+void GrpcOpenTelemetryAccessLoggerImpl::initMessage() {}
 
 void GrpcOpenTelemetryAccessLoggerImpl::clearMessage() { root_->clear_logs(); }
 

source/extensions/access_loggers/grpc/grpc_ot_access_log_impl.h

@@ -39,16 +39,14 @@ class GrpcOpenTelemetryAccessLoggerImpl
                                     envoy::config::core::v3::ApiVersion transport_api_version);
 
 private:
-  void initRoot();
+  void initMessageRoot(const std::string& log_name, const LocalInfo::LocalInfo& local_info);
   // Extensions::AccessLoggers::GrpcCommon::GrpcAccessLogger
   void addEntry(opentelemetry::proto::logs::v1::LogRecord&& entry) override;
   void addEntry(opentelemetry::proto::logs::v1::ResourceLogs&& entry) override;
   bool isEmpty() override;
   void initMessage() override;
   void clearMessage() override;
 
-  const std::string log_name_;
-  const LocalInfo::LocalInfo& local_info_;
   opentelemetry::proto::logs::v1::InstrumentationLibraryLogs* root_;
 };
 
@@ -57,10 +55,6 @@ class GrpcOpenTelemetryAccessLoggerCacheImpl
           GrpcOpenTelemetryAccessLoggerImpl,
           envoy::extensions::access_loggers::grpc::v3::CommonGrpcAccessLogConfig> {
 public:
-  // Inherited constructor.
-  // using Common::GrpcAccessLoggerCache<GrpcOpenTelemetryAccessLoggerImpl,
-  //                                     envoy::extensions::access_loggers::grpc::v3::
-  //                                         CommonGrpcAccessLogConfig>::GrpcAccessLoggerCache;
   GrpcOpenTelemetryAccessLoggerCacheImpl(Grpc::AsyncClientManager& async_client_manager,
                                          Stats::Scope& scope, ThreadLocal::SlotAllocator& tls,
                                          const LocalInfo::LocalInfo& local_info);

test/extensions/access_loggers/grpc/grpc_ot_access_log_impl_test.cc

@@ -21,11 +21,7 @@ using testing::_;
 using testing::Invoke;
 using testing::NiceMock;
 using testing::Return;
-
-// opentelemetry::proto::logs::v1::LogRecord,
-// opentelemetry::proto::logs::v1::ResourceLogs /*TCP*/,
-// opentelemetry::proto::collector::logs::v1::ExportLogsServiceRequest,
-// opentelemetry::proto::collector::logs::v1::ExportLogsServiceResponse>
+using testing::ReturnRef;
 
 namespace Envoy {
 namespace Extensions {
@@ -35,6 +31,9 @@ namespace {
 
 constexpr std::chrono::milliseconds FlushInterval(10);
 constexpr int BUFFER_SIZE_BYTES = 0;
+const std::string ZONE_NAME = "zone_name";
+const std::string CLUSTER_NAME = "cluster_name";
+const std::string NODE_NAME = "node_name";
 
 // A helper test class to mock and intercept GrpcOpenTelemetryAccessLoggerImpl streams.
 class GrpcOpenTelemetryAccessLoggerImplTestHelper {
@@ -45,8 +44,9 @@ class GrpcOpenTelemetryAccessLoggerImplTestHelper {
 
   GrpcOpenTelemetryAccessLoggerImplTestHelper(LocalInfo::MockLocalInfo& local_info,
                                               Grpc::MockAsyncClient* async_client) {
-    // EXPECT_CALL(local_info, node());
-    (void)local_info;
+    EXPECT_CALL(local_info, zoneName()).WillOnce(ReturnRef(ZONE_NAME));
+    EXPECT_CALL(local_info, clusterName()).WillOnce(ReturnRef(CLUSTER_NAME));
+    EXPECT_CALL(local_info, nodeName()).WillOnce(ReturnRef(NODE_NAME));
     EXPECT_CALL(*async_client, startRaw(_, _, _, _))
         .WillOnce(
             Invoke([this](absl::string_view, absl::string_view, Grpc::RawAsyncStreamCallbacks& cbs,
@@ -97,11 +97,22 @@ class GrpcOpenTelemetryAccessLoggerImplTest : public testing::Test {
 TEST_F(GrpcOpenTelemetryAccessLoggerImplTest, LogHttp) {
   grpc_access_logger_impl_test_helper_.expectStreamMessage(R"EOF(
   resource_logs:
-    - instrumentation_library_logs:
-      - instrumentation_library:
-          name: "envoy"
-          version: "v1"
-        logs:
+    resource:
+      attributes:
+        - key: "log_name"
+          value:
+            string_value: "test_log_name"
+        - key: "zone_name"
+          value:
+            string_value: "zone_name"
+        - key: "cluster_name"
+          value:
+            string_value: "cluster_name"
+        - key: "node_name"
+          value:
+            string_value: "node_name"
+    instrumentation_library_logs:
+      - logs:
           - severity_text: "test-severity-text"
   )EOF");
   opentelemetry::proto::logs::v1::LogRecord entry;
@@ -112,11 +123,22 @@ TEST_F(GrpcOpenTelemetryAccessLoggerImplTest, LogHttp) {
 TEST_F(GrpcOpenTelemetryAccessLoggerImplTest, LogTcp) {
   grpc_access_logger_impl_test_helper_.expectStreamMessage(R"EOF(
   resource_logs:
-    - instrumentation_library_logs:
-      - instrumentation_library:
-          name: "envoy"
-          version: "v1"
-        logs:
+    resource:
+      attributes:
+        - key: "log_name"
+          value:
+            string_value: "test_log_name"
+        - key: "zone_name"
+          value:
+            string_value: "zone_name"
+        - key: "cluster_name"
+          value:
+            string_value: "cluster_name"
+        - key: "node_name"
+          value:
+            string_value: "node_name"
+    instrumentation_library_logs:
+      - logs:
           - severity_text: "test-severity-text"
   )EOF");
   opentelemetry::proto::logs::v1::LogRecord entry;
@@ -161,11 +183,22 @@ TEST_F(GrpcOpenTelemetryAccessLoggerCacheImplTest, LoggerCreation) {
       logger_cache_.getOrCreateLogger(config, Common::GrpcAccessLoggerType::HTTP, scope_);
   grpc_access_logger_impl_test_helper_.expectStreamMessage(R"EOF(
   resource_logs:
-    - instrumentation_library_logs:
-      - instrumentation_library:
-          name: "envoy"
-          version: "v1"
-        logs:
+    resource:
+      attributes:
+        - key: "log_name"
+          value:
+            string_value: "test-log"
+        - key: "zone_name"
+          value:
+            string_value: "zone_name"
+        - key: "cluster_name"
+          value:
+            string_value: "cluster_name"
+        - key: "node_name"
+          value:
+            string_value: "node_name"
+    instrumentation_library_logs:
+      - logs:
           - severity_text: "test-severity-text"
   )EOF");
   opentelemetry::proto::logs::v1::LogRecord entry;