Skip to content

Commit 66ef4ca

Browse files
yashyktcopybara-github
authored andcommitted
Stabilize call tracer transport fix experiment (grpc#39465)
Closes grpc#39465 COPYBARA_INTEGRATE_REVIEW=grpc#39465 from yashykt:RemoveCallTracerTransportFix 2f338a5 PiperOrigin-RevId: 755058234
1 parent 447644f commit 66ef4ca

File tree

9 files changed

+25
-105
lines changed

9 files changed

+25
-105
lines changed

bazel/experiments.bzl

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1478,22 +1478,12 @@ static void log_metadata(const grpc_metadata_batch* md_batch, uint32_t id,
14781478
}
14791479

14801480
static void trace_annotations(grpc_chttp2_stream* s) {
1481-
if (!grpc_core::IsCallTracerTransportFixEnabled()) {
1482-
if (s->parent_call_tracer != nullptr) {
1483-
s->parent_call_tracer->RecordAnnotation(
1484-
grpc_core::HttpAnnotation(grpc_core::HttpAnnotation::Type::kStart,
1485-
gpr_now(GPR_CLOCK_REALTIME))
1486-
.Add(s->t->flow_control.stats())
1487-
.Add(s->flow_control.stats()));
1488-
}
1489-
} else {
1490-
if (s->call_tracer != nullptr && s->call_tracer->IsSampled()) {
1491-
s->call_tracer->RecordAnnotation(
1492-
grpc_core::HttpAnnotation(grpc_core::HttpAnnotation::Type::kStart,
1493-
gpr_now(GPR_CLOCK_REALTIME))
1494-
.Add(s->t->flow_control.stats())
1495-
.Add(s->flow_control.stats()));
1496-
}
1481+
if (s->call_tracer != nullptr && s->call_tracer->IsSampled()) {
1482+
s->call_tracer->RecordAnnotation(
1483+
grpc_core::HttpAnnotation(grpc_core::HttpAnnotation::Type::kStart,
1484+
gpr_now(GPR_CLOCK_REALTIME))
1485+
.Add(s->t->flow_control.stats())
1486+
.Add(s->flow_control.stats()));
14971487
}
14981488
}
14991489

@@ -1742,9 +1732,6 @@ static void perform_stream_op_locked(void* stream_op,
17421732
grpc_chttp2_transport* t = s->t.get();
17431733

17441734
s->traced = op->is_traced;
1745-
if (!grpc_core::IsCallTracerTransportFixEnabled()) {
1746-
s->parent_call_tracer = ParentCallTracerIfSampled(s);
1747-
}
17481735
// Some server filters populate CallTracerInterface in the context only after
17491736
// reading initial metadata. (Client-side population is done by
17501737
// client_channel filter.)

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -981,10 +981,7 @@ grpc_error_handle grpc_chttp2_header_parser_parse(void* hpack_parser,
981981
if (s != nullptr) {
982982
s->call_tracer_wrapper.RecordIncomingBytes(
983983
{0, 0, GRPC_SLICE_LENGTH(slice)});
984-
call_tracer =
985-
grpc_core::IsCallTracerTransportFixEnabled()
986-
? s->call_tracer
987-
: s->arena->GetContext<grpc_core::CallTracerAnnotationInterface>();
984+
call_tracer = s->call_tracer;
988985
}
989986
grpc_core::SharedBitGen g;
990987
grpc_error_handle error =

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

Lines changed: 16 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -491,30 +491,16 @@ class StreamWriteContext {
491491
grpc_chttp2_complete_closure_step(t_, &s_->send_initial_metadata_finished,
492492
absl::OkStatus(),
493493
"send_initial_metadata_finished");
494-
if (!grpc_core::IsCallTracerTransportFixEnabled()) {
495-
if (s_->parent_call_tracer != nullptr) {
496-
grpc_core::HttpAnnotation::WriteStats write_stats;
497-
write_stats.target_write_size = write_context_->target_write_size();
498-
s_->parent_call_tracer->RecordAnnotation(
499-
grpc_core::HttpAnnotation(
500-
grpc_core::HttpAnnotation::Type::kHeadWritten,
501-
gpr_now(GPR_CLOCK_REALTIME))
502-
.Add(s_->t->flow_control.stats())
503-
.Add(s_->flow_control.stats())
504-
.Add(write_stats));
505-
}
506-
} else {
507-
if (s_->call_tracer != nullptr && s_->call_tracer->IsSampled()) {
508-
grpc_core::HttpAnnotation::WriteStats write_stats;
509-
write_stats.target_write_size = write_context_->target_write_size();
510-
s_->call_tracer->RecordAnnotation(
511-
grpc_core::HttpAnnotation(
512-
grpc_core::HttpAnnotation::Type::kHeadWritten,
513-
gpr_now(GPR_CLOCK_REALTIME))
514-
.Add(s_->t->flow_control.stats())
515-
.Add(s_->flow_control.stats())
516-
.Add(write_stats));
517-
}
494+
if (s_->call_tracer != nullptr && s_->call_tracer->IsSampled()) {
495+
grpc_core::HttpAnnotation::WriteStats write_stats;
496+
write_stats.target_write_size = write_context_->target_write_size();
497+
s_->call_tracer->RecordAnnotation(
498+
grpc_core::HttpAnnotation(
499+
grpc_core::HttpAnnotation::Type::kHeadWritten,
500+
gpr_now(GPR_CLOCK_REALTIME))
501+
.Add(s_->t->flow_control.stats())
502+
.Add(s_->flow_control.stats())
503+
.Add(write_stats));
518504
}
519505
}
520506

@@ -670,22 +656,12 @@ class StreamWriteContext {
670656
}
671657
grpc_chttp2_mark_stream_closed(t_, s_, !t_->is_client, true,
672658
absl::OkStatus());
673-
if (!grpc_core::IsCallTracerTransportFixEnabled()) {
674-
if (s_->parent_call_tracer != nullptr) {
675-
s_->parent_call_tracer->RecordAnnotation(
676-
grpc_core::HttpAnnotation(grpc_core::HttpAnnotation::Type::kEnd,
677-
gpr_now(GPR_CLOCK_REALTIME))
678-
.Add(s_->t->flow_control.stats())
679-
.Add(s_->flow_control.stats()));
680-
}
681-
} else {
682-
if (s_->call_tracer != nullptr && s_->call_tracer->IsSampled()) {
683-
s_->call_tracer->RecordAnnotation(
684-
grpc_core::HttpAnnotation(grpc_core::HttpAnnotation::Type::kEnd,
685-
gpr_now(GPR_CLOCK_REALTIME))
686-
.Add(s_->t->flow_control.stats())
687-
.Add(s_->flow_control.stats()));
688-
}
659+
if (s_->call_tracer != nullptr && s_->call_tracer->IsSampled()) {
660+
s_->call_tracer->RecordAnnotation(
661+
grpc_core::HttpAnnotation(grpc_core::HttpAnnotation::Type::kEnd,
662+
gpr_now(GPR_CLOCK_REALTIME))
663+
.Add(s_->t->flow_control.stats())
664+
.Add(s_->flow_control.stats()));
689665
}
690666
}
691667

src/core/lib/experiments/experiments.cc

Lines changed: 0 additions & 15 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/core/lib/experiments/experiments.h

Lines changed: 0 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/core/lib/experiments/experiments.yaml

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,6 @@
5151
5252
test_tags: []
5353
allow_in_fuzzing_config: false
54-
- name: call_tracer_transport_fix
55-
description: Use the correct call tracer in transport
56-
expiry: 2025/06/01
57-
58-
test_tags: []
5954
- name: callv3_client_auth_filter
6055
description: Use the CallV3 client auth filter.
6156
expiry: 2025/06/01

src/core/lib/experiments/rollouts.yaml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@
4444
default: true
4545
- name: call_tracer_in_transport
4646
default: true
47-
- name: call_tracer_transport_fix
48-
default: true
4947
- name: chaotic_good_framing_layer
5048
default: true
5149
- name: error_flatten

test/cpp/ext/filters/census/stats_plugin_end2end_test.cc

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -813,10 +813,7 @@ TEST_F(StatsPluginEnd2EndTest, TestMetadataSizeAnnotations) {
813813
traces_recorder_->StopRecording();
814814
// Check presence of metadata size annotations in client span.
815815
auto sent_span_data = GetSpanByName(
816-
recorded_spans,
817-
absl::StrCat(
818-
grpc_core::IsCallTracerTransportFixEnabled() ? "Attempt." : "Sent.",
819-
client_method_name_));
816+
recorded_spans, absl::StrCat("Attempt.", client_method_name_));
820817
ASSERT_NE(sent_span_data, recorded_spans.end());
821818
EXPECT_TRUE(IsAnnotationPresent(
822819
sent_span_data,
@@ -856,10 +853,7 @@ TEST_F(StatsPluginEnd2EndTest, TestHttpAnnotations) {
856853
auto recorded_spans = traces_recorder_->GetAndClearSpans(3);
857854
traces_recorder_->StopRecording();
858855
auto client_span_data = GetSpanByName(
859-
recorded_spans,
860-
absl::StrCat(
861-
grpc_core::IsCallTracerTransportFixEnabled() ? "Attempt." : "Sent.",
862-
client_method_name_));
856+
recorded_spans, absl::StrCat("Attempt.", client_method_name_));
863857
ASSERT_NE(client_span_data, recorded_spans.end());
864858
EXPECT_TRUE(IsAnnotationPresent(client_span_data,
865859
"HttpAnnotation type: Start time: .* "

0 commit comments

Comments
 (0)