[PH2][ChannelZ][ZTrace][Skeleton]

What this does :
1. Basic skeleton for ChannelZ stuff in Http2ClientTransport
2. Basic plumbing for the channel args to enable ChannelZ in Http2ClientTransport tests, end to end tests and Connector code
What this does not do :
1. Exhaustive list of everything that we want to trace. A lot needs to be done here. But we will gradually build this as we debug. We will organically build this.
2. Some of the tests will not include this stuff in the post mortems. We will look at that too.

PiperOrigin-RevId: 812651089

Author: tanvi-jagtap

src/core/ext/transport/chttp2/transport/http2_client_transport.cc

@@ -39,6 +39,7 @@
 #include "src/core/ext/transport/chttp2/transport/http2_settings.h"
 #include "src/core/ext/transport/chttp2/transport/http2_settings_manager.h"
 #include "src/core/ext/transport/chttp2/transport/http2_status.h"
+#include "src/core/ext/transport/chttp2/transport/http2_ztrace_collector.h"
 #include "src/core/ext/transport/chttp2/transport/internal_channel_arg_names.h"
 #include "src/core/ext/transport/chttp2/transport/message_assembler.h"
 #include "src/core/ext/transport/chttp2/transport/stream_data_queue.h"
@@ -699,7 +700,11 @@ auto Http2ClientTransport::WriteControlFrames() {
         return self->endpoint_.Write(std::move(output_buf),
                                      PromiseEndpoint::WriteArgs{});
       },
-      [] { return absl::OkStatus(); });
+      [self = RefAsSubclass<Http2ClientTransport>(), buffer_length] {
+        self->ztrace_collector_->Append(
+            PromiseEndpointWriteTrace{buffer_length});
+        return absl::OkStatus();
+      });
 }
 
 void Http2ClientTransport::NotifyControlFramesWriteDone() {
@@ -908,7 +913,9 @@ Http2ClientTransport::Http2ClientTransport(
     PromiseEndpoint endpoint, GRPC_UNUSED const ChannelArgs& channel_args,
     std::shared_ptr<EventEngine> event_engine,
     grpc_closure* on_receive_settings)
-    : endpoint_(std::move(endpoint)),
+    : channelz::DataSource(http2::CreateChannelzSocketNode(
+          endpoint.GetEventEngineEndpoint(), channel_args)),
+      endpoint_(std::move(endpoint)),
       stream_id_mutex_(/*Initial Stream Id*/ 1),
       should_reset_ping_clock_(false),
       incoming_header_in_progress_(false),
@@ -959,8 +966,10 @@ Http2ClientTransport::Http2ClientTransport(
       flow_control_(
           "PH2_Client",
           channel_args.GetBool(GRPC_ARG_HTTP2_BDP_PROBE).value_or(true),
-          &memory_owner_) {
+          &memory_owner_),
+      ztrace_collector_(std::make_shared<PromiseHttp2ZTraceCollector>()) {
   GRPC_HTTP2_CLIENT_DLOG << "Http2ClientTransport Constructor Begin";
+  SourceConstructed();
 
   InitLocalSettings(settings_.mutable_local(), /*is_client=*/true);
   ReadSettingsFromChannelArgs(channel_args, settings_.mutable_local(),
@@ -1190,9 +1199,24 @@ Http2ClientTransport::~Http2ClientTransport() {
   GRPC_HTTP2_CLIENT_DLOG << "Http2ClientTransport Destructor Begin";
   GRPC_DCHECK(stream_list_.empty());
   memory_owner_.Reset();
+  SourceDestructing();
   GRPC_HTTP2_CLIENT_DLOG << "Http2ClientTransport Destructor End";
 }
 
+void Http2ClientTransport::AddData(channelz::DataSink sink) {
+  sink.AddData(
+      "Http2ClientTransport",
+      channelz::PropertyList()
+          .Set("settings", settings_.ChannelzProperties())
+          .Set("keepalive_time", keepalive_time_)
+          .Set("keepalive_timeout", keepalive_timeout_)
+          .Set("ping_timeout", ping_timeout_)
+          .Set("keepalive_permit_without_calls",
+               keepalive_permit_without_calls_)
+          .Set("flow_control", flow_control_.stats().ChannelzProperties()));
+  general_party_->ExportToChannelz("Http2ClientTransport Party", sink);
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // Stream Related Operations
 

src/core/ext/transport/chttp2/transport/http2_client_transport.h

@@ -35,6 +35,7 @@
 #include "src/core/ext/transport/chttp2/transport/http2_settings_promises.h"
 #include "src/core/ext/transport/chttp2/transport/http2_status.h"
 #include "src/core/ext/transport/chttp2/transport/http2_transport.h"
+#include "src/core/ext/transport/chttp2/transport/http2_ztrace_collector.h"
 #include "src/core/ext/transport/chttp2/transport/keepalive.h"
 #include "src/core/ext/transport/chttp2/transport/message_assembler.h"
 #include "src/core/ext/transport/chttp2/transport/ping_promise.h"
@@ -90,7 +91,8 @@ namespace http2 {
 // familiar with the PH2 project (Moving chttp2 to promises.)
 // TODO(tjagtap) : [PH2][P3] : Update the experimental status of the code before
 // http2 rollout begins.
-class Http2ClientTransport final : public ClientTransport {
+class Http2ClientTransport final : public ClientTransport,
+                                   public channelz::DataSource {
   // TODO(tjagtap) : [PH2][P3] Move the definitions to the header for better
   // inlining. For now definitions are in the cc file to
   // reduce cognitive load in the header.
@@ -130,9 +132,18 @@ class Http2ClientTransport final : public ClientTransport {
   void AbortWithError();
 
   RefCountedPtr<channelz::SocketNode> GetSocketNode() const override {
+    return const_cast<channelz::BaseNode*>(
+               channelz::DataSource::channelz_node())
+        ->RefAsSubclass<channelz::SocketNode>();
+  }
+
+  std::unique_ptr<channelz::ZTrace> GetZTrace(absl::string_view name) override {
+    if (name == "transport_frames") return ztrace_collector_->MakeZTrace();
     return nullptr;
   }
 
+  void AddData(channelz::DataSink sink) override;
+
   auto TestOnlyTriggerWriteCycle() {
     return Immediate(writable_stream_list_.ForceReadyForWrite());
   }
@@ -311,15 +322,13 @@ class Http2ClientTransport final : public ClientTransport {
 
   auto EndpointReadSlice(const size_t num_bytes) {
     return Map(endpoint_.ReadSlice(num_bytes),
-               [self = RefAsSubclass<Http2ClientTransport>()](
-                   absl::StatusOr<Slice> status) {
-                 // We are ignoring the case where the read fails and call
-                 // GotData() regardless. Reasoning:
-                 // 1. It is expected that if the read fails, the transport will
-                 //    close and the keepalive loop will be stopped.
-                 // 2. It does not seem worth to have an extra condition for the
-                 //    success cases which would be way more common.
-                 self->keepalive_manager_.GotData();
+               [self = RefAsSubclass<Http2ClientTransport>(),
+                num_bytes](absl::StatusOr<Slice> status) {
+                 if (status.ok()) {
+                   self->keepalive_manager_.GotData();
+                   self->ztrace_collector_->Append(
+                       PromiseEndpointReadTrace{num_bytes});
+                 }
                  return status;
                });
   }
@@ -351,15 +360,13 @@ class Http2ClientTransport final : public ClientTransport {
 
   auto EndpointRead(const size_t num_bytes) {
     return Map(endpoint_.Read(num_bytes),
-               [self = RefAsSubclass<Http2ClientTransport>()](
-                   absl::StatusOr<SliceBuffer> status) {
-                 // We are ignoring the case where the read fails and call
-                 // GotData() regardless. Reasoning:
-                 // 1. It is expected that if the read fails, the transport will
-                 //    close and the keepalive loop will be stopped.
-                 // 2. It does not seem worth to have an extra condition for the
-                 //    success cases which would be way more common.
-                 self->keepalive_manager_.GotData();
+               [self = RefAsSubclass<Http2ClientTransport>(),
+                num_bytes](absl::StatusOr<SliceBuffer> status) {
+                 if (status.ok()) {
+                   self->keepalive_manager_.GotData();
+                   self->ztrace_collector_->Append(
+                       PromiseEndpointReadTrace{num_bytes});
+                 }
                  return status;
                });
   }
@@ -611,6 +618,7 @@ class Http2ClientTransport final : public ClientTransport {
   GRPC_UNUSED bool enable_preferred_rx_crypto_frame_advertisement_;
   MemoryOwner memory_owner_;
   chttp2::TransportFlowControl flow_control_;
+  std::shared_ptr<PromiseHttp2ZTraceCollector> ztrace_collector_;
 };
 
 // Since the corresponding class in CHTTP2 is about 3.9KB, our goal is to

src/core/ext/transport/chttp2/transport/http2_transport.cc

@@ -23,6 +23,7 @@
 
 #include "src/core/call/call_spine.h"
 #include "src/core/call/metadata_info.h"
+#include "src/core/channelz/channelz.h"
 #include "src/core/ext/transport/chttp2/transport/flow_control.h"
 #include "src/core/ext/transport/chttp2/transport/frame.h"
 #include "src/core/lib/promise/mpsc.h"
@@ -123,5 +124,26 @@ void ReadSettingsFromChannelArgs(const ChannelArgs& channel_args,
       << "}";
 }
 
+RefCountedPtr<channelz::SocketNode> CreateChannelzSocketNode(
+    std::shared_ptr<grpc_event_engine::experimental::EventEngine::Endpoint>
+        event_engine_endpoint,
+    const ChannelArgs& args) {
+  if (args.GetBool(GRPC_ARG_ENABLE_CHANNELZ)
+          .value_or(GRPC_ENABLE_CHANNELZ_DEFAULT)) {
+    auto local_addr = grpc_event_engine::experimental::ResolvedAddressToString(
+        event_engine_endpoint->GetLocalAddress());
+    auto peer_addr = grpc_event_engine::experimental::ResolvedAddressToString(
+        event_engine_endpoint->GetPeerAddress());
+    GRPC_HTTP2_COMMON_DLOG << "CreateChannelzSocketNode: local_addr: "
+                           << local_addr.value_or("unknown")
+                           << " peer_addr: " << peer_addr.value_or("unknown");
+    return MakeRefCounted<channelz::SocketNode>(
+        local_addr.value_or("unknown"), peer_addr.value_or("unknown"),
+        absl::StrCat("http2", " ", peer_addr.value_or("unknown")),
+        args.GetObjectRef<channelz::SocketNode::Security>());
+  }
+  return nullptr;
+}
+
 }  // namespace http2
 }  // namespace grpc_core

src/core/ext/transport/chttp2/transport/http2_transport.h

@@ -24,9 +24,11 @@
 
 #include "src/core/call/call_spine.h"
 #include "src/core/call/metadata_info.h"
+#include "src/core/channelz/channelz.h"
 #include "src/core/ext/transport/chttp2/transport/flow_control.h"
 #include "src/core/ext/transport/chttp2/transport/frame.h"
 #include "src/core/ext/transport/chttp2/transport/http2_settings.h"
+#include "src/core/lib/event_engine/tcp_socket_utils.h"
 #include "src/core/lib/promise/mpsc.h"
 #include "src/core/lib/promise/party.h"
 #include "src/core/lib/transport/promise_endpoint.h"
@@ -70,6 +72,11 @@ void ReadSettingsFromChannelArgs(const ChannelArgs& channel_args,
                                  chttp2::TransportFlowControl& flow_control,
                                  const bool is_client);
 
+RefCountedPtr<channelz::SocketNode> CreateChannelzSocketNode(
+    std::shared_ptr<grpc_event_engine::experimental::EventEngine::Endpoint>
+        event_engine_endpoint,
+    const ChannelArgs& args);
+
 }  // namespace http2
 }  // namespace grpc_core
 

src/core/ext/transport/chttp2/transport/http2_ztrace_collector.h

@@ -19,6 +19,7 @@
 #include <map>
 #include <string>
 
+#include "src/core/channelz/property_list.h"
 #include "src/core/channelz/ztrace_collector.h"
 #include "src/core/ext/transport/chttp2/transport/frame.h"
 
@@ -230,6 +231,37 @@ using Http2ZTraceCollector = channelz::ZTraceCollector<
     H2UnknownFrameTrace, H2FlowControlStall, H2BeginWriteCycle, H2EndWriteCycle,
     H2BeginEndpointWrite>;
 
+struct PromiseEndpointReadTrace {
+  uint64_t bytes;
+  channelz::PropertyList ChannelzProperties() const {
+    return channelz::PropertyList().Set("read_bytes", bytes);
+  }
+};
+
+struct PromiseEndpointWriteTrace {
+  uint64_t bytes;
+  channelz::PropertyList ChannelzProperties() const {
+    return channelz::PropertyList().Set("written_bytes", bytes);
+  }
+};
+
+namespace promise_http2_ztrace_collector_detail {
+class Config {
+ public:
+  explicit Config(const channelz::ZTrace::Args&) {}
+
+  template <typename T>
+  bool Finishes(const T&) {
+    return false;
+  }
+};
+}  // namespace promise_http2_ztrace_collector_detail
+
+using PromiseHttp2ZTraceCollector =
+    channelz::ZTraceCollector<promise_http2_ztrace_collector_detail::Config,
+                              PromiseEndpointReadTrace,
+                              PromiseEndpointWriteTrace>;
+
 }  // namespace grpc_core
 
 #endif  // GRPC_SRC_CORE_EXT_TRANSPORT_CHTTP2_TRANSPORT_HTTP2_ZTRACE_COLLECTOR_H

test/core/end2end/end2end_ph2_config.cc

@@ -34,6 +34,23 @@
 
 namespace grpc_core {
 
+class Ph2InsecureFixture : public InsecureFixture {
+ public:
+  Ph2InsecureFixture() {
+    // At Least one of the 2 peers MUST be a PH2
+    GRPC_DCHECK(IsPromiseBasedHttp2ClientTransportEnabled() ||
+                IsPromiseBasedHttp2ServerTransportEnabled());
+  }
+
+  ChannelArgs MutateClientArgs(ChannelArgs args) override {
+    return args.Set(GRPC_ARG_ENABLE_CHANNELZ, true);
+  }
+
+  ChannelArgs MutateServerArgs(ChannelArgs args) override {
+    return args.Set(GRPC_ARG_ENABLE_CHANNELZ, true);
+  }
+};
+
 #define GRPC_HTTP2_PROMISE_CLIENT_TRANSPORT_AVOID_LIST                         \
   "CoreClientChannelTests.DeadlineAfterAcceptWithServiceConfig"                \
   "|CoreClientChannelTests.DeadlineAfterRoundTripWithServiceConfig"            \
@@ -137,7 +154,7 @@ std::vector<CoreTestConfiguration> End2endTestConfigs() {
         /*create_fixture=*/
         [](const ChannelArgs& /*client_args*/,
            const ChannelArgs& /*server_args*/) {
-          return std::make_unique<InsecureFixture>();
+          return std::make_unique<Ph2InsecureFixture>();
         },
         /* include_test_suites */
         GRPC_HTTP2_PROMISE_CLIENT_TRANSPORT_ALLOW_SUITE,