From 5ac8442432950c5c11a39904bb1f381829b7ae0c Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 29 Nov 2023 09:41:04 -0800 Subject: [PATCH 01/20] [copybara] omit bot from autofix (#35141) --- .github/workflows/pr-auto-fix.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr-auto-fix.yaml b/.github/workflows/pr-auto-fix.yaml index 573de52d9cb..2374a5e0e0d 100644 --- a/.github/workflows/pr-auto-fix.yaml +++ b/.github/workflows/pr-auto-fix.yaml @@ -48,7 +48,7 @@ jobs: with: script: | // If you'd like not to run this code on your commits, add your github user id here: - NO_AUTOFIX_USERS = [] + NO_AUTOFIX_USERS = ["copybara-service"] const { owner, repo } = context.repo if (NO_AUTOFIX_USERS.includes(context.actor)) { console.log('Cancelling'); From 492dfba681e50000ea81b9fbf705625d268686bc Mon Sep 17 00:00:00 2001 From: Paulo Castello da Costa Date: Wed, 29 Nov 2023 10:57:16 -0800 Subject: [PATCH 02/20] [benchmark] Exclude dotnet from OSS benchmarks CI. (#35068) QPS worker build is currently failing. Closes #35068 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35068 from paulosjca:dotnet cc4ef42659651aa8fa9fc9ad6b3c5c42b2dc76d5 PiperOrigin-RevId: 586393834 --- tools/internal_ci/linux/grpc_e2e_performance_gke.sh | 6 +++--- .../linux/grpc_e2e_performance_gke_experiment.sh | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/internal_ci/linux/grpc_e2e_performance_gke.sh b/tools/internal_ci/linux/grpc_e2e_performance_gke.sh index 15951cccc75..a8da8a9fae6 100755 --- a/tools/internal_ci/linux/grpc_e2e_performance_gke.sh +++ b/tools/internal_ci/linux/grpc_e2e_performance_gke.sh @@ -131,9 +131,9 @@ configLangArgs32core+=( -l c++ ) runnerLangArgs+=( -l "cxx:${GRPC_CORE_REPO}:${GRPC_CORE_COMMIT}" ) # dotnet -configLangArgs8core+=( -l dotnet ) -configLangArgs32core+=( -l dotnet ) -runnerLangArgs+=( -l "dotnet:${GRPC_DOTNET_REPO}:${GRPC_DOTNET_COMMIT}" ) +# configLangArgs8core+=( -l dotnet ) +# configLangArgs32core+=( -l dotnet ) +# runnerLangArgs+=( -l "dotnet:${GRPC_DOTNET_REPO}:${GRPC_DOTNET_COMMIT}" ) # # go configLangArgs8core+=( -l go ) diff --git a/tools/internal_ci/linux/grpc_e2e_performance_gke_experiment.sh b/tools/internal_ci/linux/grpc_e2e_performance_gke_experiment.sh index f62d9920b72..c3b99af80eb 100755 --- a/tools/internal_ci/linux/grpc_e2e_performance_gke_experiment.sh +++ b/tools/internal_ci/linux/grpc_e2e_performance_gke_experiment.sh @@ -125,9 +125,9 @@ configLangArgs32core+=( -l c++ ) runnerLangArgs+=( -l "cxx:${GRPC_CORE_REPO}:${GRPC_CORE_COMMIT}" ) # dotnet -configLangArgs8core+=( -l dotnet ) -configLangArgs32core+=( -l dotnet ) -runnerLangArgs+=( -l "dotnet:${GRPC_DOTNET_REPO}:${GRPC_DOTNET_COMMIT}" ) +# configLangArgs8core+=( -l dotnet ) +# configLangArgs32core+=( -l dotnet ) +# runnerLangArgs+=( -l "dotnet:${GRPC_DOTNET_REPO}:${GRPC_DOTNET_COMMIT}" ) # # go configLangArgs8core+=( -l go ) From 57d8462aee47ead0d2cba51ee7d516a8ab6c6d83 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 29 Nov 2023 11:29:46 -0800 Subject: [PATCH 03/20] Remove IWYU requirements for changes PiperOrigin-RevId: 586404245 --- .../dockerfile/grpc_iwyu/Dockerfile.template | 30 - tools/buildgen/generate_projects.sh | 2 - tools/distrib/fix_build_deps.py | 684 ------------------ tools/distrib/iwyu.sh | 63 -- tools/distrib/iwyu_mappings.imp | 9 - tools/dockerfile/grpc_iwyu/Dockerfile | 28 - tools/dockerfile/grpc_iwyu/iwyu.sh | 139 ---- tools/internal_ci/linux/grpc_iwyu.cfg | 30 - .../linux/pull_request/grpc_iwyu.cfg | 30 - tools/run_tests/run_tests.py | 1 - tools/run_tests/sanity/iwyu_tests.yaml | 2 - 11 files changed, 1018 deletions(-) delete mode 100644 templates/tools/dockerfile/grpc_iwyu/Dockerfile.template delete mode 100755 tools/distrib/fix_build_deps.py delete mode 100755 tools/distrib/iwyu.sh delete mode 100644 tools/distrib/iwyu_mappings.imp delete mode 100644 tools/dockerfile/grpc_iwyu/Dockerfile delete mode 100755 tools/dockerfile/grpc_iwyu/iwyu.sh delete mode 100644 tools/internal_ci/linux/grpc_iwyu.cfg delete mode 100644 tools/internal_ci/linux/pull_request/grpc_iwyu.cfg delete mode 100644 tools/run_tests/sanity/iwyu_tests.yaml diff --git a/templates/tools/dockerfile/grpc_iwyu/Dockerfile.template b/templates/tools/dockerfile/grpc_iwyu/Dockerfile.template deleted file mode 100644 index ecb4deef283..00000000000 --- a/templates/tools/dockerfile/grpc_iwyu/Dockerfile.template +++ /dev/null @@ -1,30 +0,0 @@ -%YAML 1.2 ---- | - # Copyright 2021 gRPC authors. - # - # Licensed under the Apache License, Version 2.0 (the "License"); - # you may not use this file except in compliance with the License. - # You may obtain a copy of the License at - # - # http://www.apache.org/licenses/LICENSE-2.0 - # - # Unless required by applicable law or agreed to in writing, software - # distributed under the License is distributed on an "AS IS" BASIS, - # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - # See the License for the specific language governing permissions and - # limitations under the License. - - # Docker file for running IWYU. - # Updated: 2022-11-03 - - FROM silkeh/clang:16-bullseye - - # Install prerequisites for the iwyu script - RUN apt-get update && apt-get install -y python3 jq git cmake python zlib1g-dev libtinfo-dev libclang-16-dev && apt-get clean - ADD iwyu.sh / - - # When running locally, we'll be impersonating the current user, so we need - # to make the script runnable by everyone. - RUN chmod a+rx /iwyu.sh - - CMD ["echo 'Run with tools/distrib/iwyu.sh'"] diff --git a/tools/buildgen/generate_projects.sh b/tools/buildgen/generate_projects.sh index 9ddd588bb37..e7905a40c95 100755 --- a/tools/buildgen/generate_projects.sh +++ b/tools/buildgen/generate_projects.sh @@ -28,8 +28,6 @@ fi cd `dirname $0`/../.. -tools/distrib/fix_build_deps.py - echo "Generating build_autogenerated.yaml from bazel BUILD file" rm -f build_autogenerated.yaml python3 tools/buildgen/extract_metadata_from_bazel_xml.py diff --git a/tools/distrib/fix_build_deps.py b/tools/distrib/fix_build_deps.py deleted file mode 100755 index 0c014ec2b76..00000000000 --- a/tools/distrib/fix_build_deps.py +++ /dev/null @@ -1,684 +0,0 @@ -#!/usr/bin/env python3 - -# Copyright 2022 gRPC authors. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import argparse -import collections -from doctest import SKIP -import multiprocessing -import os -import re -import sys - -import run_buildozer - -# find our home -ROOT = os.path.abspath(os.path.join(os.path.dirname(sys.argv[0]), "../..")) -os.chdir(ROOT) - -vendors = collections.defaultdict(list) -scores = collections.defaultdict(int) -avoidness = collections.defaultdict(int) -consumes = {} -no_update = set() -buildozer_commands = [] -original_deps = {} -original_external_deps = {} -skip_headers = collections.defaultdict(set) - -# TODO(ctiller): ideally we wouldn't hardcode a bunch of paths here. -# We can likely parse out BUILD files from dependencies to generate this index. -EXTERNAL_DEPS = { - "absl/algorithm/container.h": "absl/algorithm:container", - "absl/base/attributes.h": "absl/base:core_headers", - "absl/base/call_once.h": "absl/base", - # TODO(ctiller) remove this - "absl/base/internal/endian.h": "absl/base:endian", - "absl/base/thread_annotations.h": "absl/base:core_headers", - "absl/container/flat_hash_map.h": "absl/container:flat_hash_map", - "absl/container/flat_hash_set.h": "absl/container:flat_hash_set", - "absl/container/inlined_vector.h": "absl/container:inlined_vector", - "absl/cleanup/cleanup.h": "absl/cleanup", - "absl/debugging/failure_signal_handler.h": ( - "absl/debugging:failure_signal_handler" - ), - "absl/debugging/stacktrace.h": "absl/debugging:stacktrace", - "absl/debugging/symbolize.h": "absl/debugging:symbolize", - "absl/flags/flag.h": "absl/flags:flag", - "absl/flags/marshalling.h": "absl/flags:marshalling", - "absl/flags/parse.h": "absl/flags:parse", - "absl/functional/any_invocable.h": "absl/functional:any_invocable", - "absl/functional/bind_front.h": "absl/functional:bind_front", - "absl/functional/function_ref.h": "absl/functional:function_ref", - "absl/hash/hash.h": "absl/hash", - "absl/memory/memory.h": "absl/memory", - "absl/meta/type_traits.h": "absl/meta:type_traits", - "absl/numeric/int128.h": "absl/numeric:int128", - "absl/random/random.h": "absl/random", - "absl/random/bit_gen_ref.h": "absl/random:bit_gen_ref", - "absl/random/mocking_bit_gen.h": "absl/random:mocking_bit_gen", - "absl/random/distributions.h": "absl/random:distributions", - "absl/random/uniform_int_distribution.h": "absl/random:distributions", - "absl/status/status.h": "absl/status", - "absl/status/statusor.h": "absl/status:statusor", - "absl/strings/ascii.h": "absl/strings", - "absl/strings/cord.h": "absl/strings:cord", - "absl/strings/escaping.h": "absl/strings", - "absl/strings/match.h": "absl/strings", - "absl/strings/numbers.h": "absl/strings", - "absl/strings/str_cat.h": "absl/strings", - "absl/strings/str_format.h": "absl/strings:str_format", - "absl/strings/str_join.h": "absl/strings", - "absl/strings/str_replace.h": "absl/strings", - "absl/strings/str_split.h": "absl/strings", - "absl/strings/string_view.h": "absl/strings", - "absl/strings/strip.h": "absl/strings", - "absl/strings/substitute.h": "absl/strings", - "absl/synchronization/mutex.h": "absl/synchronization", - "absl/synchronization/notification.h": "absl/synchronization", - "absl/time/clock.h": "absl/time", - "absl/time/time.h": "absl/time", - "absl/types/optional.h": "absl/types:optional", - "absl/types/span.h": "absl/types:span", - "absl/types/variant.h": "absl/types:variant", - "absl/utility/utility.h": "absl/utility", - "address_sorting/address_sorting.h": "address_sorting", - "google/cloud/opentelemetry/resource_detector.h": "google_cloud_cpp:opentelemetry", - "opentelemetry/common/attribute_value.h": "otel/api", - "opentelemetry/common/key_value_iterable.h": "otel/api", - "opentelemetry/nostd/function_ref.h": "otel/api", - "opentelemetry/nostd/string_view.h": "otel/api", - "opentelemetry/context/context.h": "otel/api", - "opentelemetry/metrics/meter.h": "otel/api", - "opentelemetry/metrics/meter_provider.h": "otel/api", - "opentelemetry/metrics/provider.h": "otel/api", - "opentelemetry/metrics/sync_instruments.h": "otel/api", - "opentelemetry/nostd/shared_ptr.h": "otel/api", - "opentelemetry/nostd/unique_ptr.h": "otel/api", - "opentelemetry/sdk/metrics/meter_provider.h": "otel/sdk/src/metrics", - "opentelemetry/sdk/common/attribute_utils.h": "otel/sdk:headers", - "opentelemetry/sdk/resource/resource.h": "otel/sdk:headers", - "opentelemetry/sdk/resource/resource_detector.h": "otel/sdk:headers", - "opentelemetry/sdk/resource/semantic_conventions.h": "otel/sdk:headers", - "ares.h": "cares", - "fuzztest/fuzztest.h": ["fuzztest", "fuzztest_main"], - "google/api/monitored_resource.pb.h": ( - "google/api:monitored_resource_cc_proto" - ), - "google/devtools/cloudtrace/v2/tracing.grpc.pb.h": ( - "googleapis_trace_grpc_service" - ), - "google/logging/v2/logging.grpc.pb.h": "googleapis_logging_grpc_service", - "google/logging/v2/logging.pb.h": "googleapis_logging_cc_proto", - "google/logging/v2/log_entry.pb.h": "googleapis_logging_cc_proto", - "google/monitoring/v3/metric_service.grpc.pb.h": ( - "googleapis_monitoring_grpc_service" - ), - "gmock/gmock.h": "gtest", - "gtest/gtest.h": "gtest", - "opencensus/exporters/stats/stackdriver/stackdriver_exporter.h": ( - "opencensus-stats-stackdriver_exporter" - ), - "opencensus/exporters/trace/stackdriver/stackdriver_exporter.h": ( - "opencensus-trace-stackdriver_exporter" - ), - "opencensus/trace/context_util.h": "opencensus-trace-context_util", - "opencensus/trace/propagation/grpc_trace_bin.h": ( - "opencensus-trace-propagation" - ), - "opencensus/tags/context_util.h": "opencensus-tags-context_util", - "opencensus/trace/span_context.h": "opencensus-trace-span_context", - "openssl/base.h": "libssl", - "openssl/bio.h": "libssl", - "openssl/bn.h": "libcrypto", - "openssl/buffer.h": "libcrypto", - "openssl/crypto.h": "libcrypto", - "openssl/digest.h": "libssl", - "openssl/engine.h": "libcrypto", - "openssl/err.h": "libcrypto", - "openssl/evp.h": "libcrypto", - "openssl/hmac.h": "libcrypto", - "openssl/mem.h": "libcrypto", - "openssl/param_build.h": "libcrypto", - "openssl/pem.h": "libcrypto", - "openssl/rsa.h": "libcrypto", - "openssl/sha.h": "libcrypto", - "openssl/ssl.h": "libssl", - "openssl/tls1.h": "libssl", - "openssl/x509.h": "libcrypto", - "openssl/x509v3.h": "libcrypto", - "re2/re2.h": "re2", - "upb/base/string_view.h": "upb_base_lib", - "upb/collections/map.h": "upb_collections_lib", - "upb/reflection/def.h": "upb_reflection", - "upb/json/encode.h": "upb_json_lib", - "upb/mem/arena.h": "upb_mem_lib", - "upb/text/encode.h": "upb_textformat_lib", - "upb/reflection/def.hpp": "upb_reflection", - "upb/upb.h": "upb_amalgamation_lib", - "upb/upb.hpp": "upb_lib", - "xxhash.h": "xxhash", - "zlib.h": "madler_zlib", -} - -INTERNAL_DEPS = { - "test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.h": ( - "//test/core/event_engine/fuzzing_event_engine" - ), - "test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.pb.h": "//test/core/event_engine/fuzzing_event_engine:fuzzing_event_engine_proto", - "test/core/experiments/test_experiments.h": "//test/core/experiments:test_experiments_lib", - "google/api/expr/v1alpha1/syntax.upb.h": "google_api_expr_v1alpha1_syntax_upb", - "google/rpc/status.upb.h": "google_rpc_status_upb", - "google/protobuf/any.upb.h": "protobuf_any_upb", - "google/protobuf/duration.upb.h": "protobuf_duration_upb", - "google/protobuf/struct.upb.h": "protobuf_struct_upb", - "google/protobuf/timestamp.upb.h": "protobuf_timestamp_upb", - "google/protobuf/wrappers.upb.h": "protobuf_wrappers_upb", - "grpc/status.h": "grpc_public_hdrs", - "src/proto/grpc/channelz/channelz.grpc.pb.h": ( - "//src/proto/grpc/channelz:channelz_proto" - ), - "src/proto/grpc/core/stats.pb.h": "//src/proto/grpc/core:stats_proto", - "src/proto/grpc/health/v1/health.upb.h": "grpc_health_upb", - "src/proto/grpc/lb/v1/load_reporter.grpc.pb.h": ( - "//src/proto/grpc/lb/v1:load_reporter_proto" - ), - "src/proto/grpc/lb/v1/load_balancer.upb.h": "grpc_lb_upb", - "src/proto/grpc/reflection/v1alpha/reflection.grpc.pb.h": ( - "//src/proto/grpc/reflection/v1alpha:reflection_proto" - ), - "src/proto/grpc/gcp/transport_security_common.upb.h": "alts_upb", - "src/proto/grpc/gcp/handshaker.upb.h": "alts_upb", - "src/proto/grpc/gcp/altscontext.upb.h": "alts_upb", - "src/proto/grpc/lookup/v1/rls.upb.h": "rls_upb", - "src/proto/grpc/lookup/v1/rls_config.upb.h": "rls_config_upb", - "src/proto/grpc/lookup/v1/rls_config.upbdefs.h": "rls_config_upbdefs", - "src/proto/grpc/testing/xds/v3/csds.grpc.pb.h": ( - "//src/proto/grpc/testing/xds/v3:csds_proto" - ), - "xds/data/orca/v3/orca_load_report.upb.h": "xds_orca_upb", - "xds/service/orca/v3/orca.upb.h": "xds_orca_service_upb", - "xds/type/v3/typed_struct.upb.h": "xds_type_upb", -} - - -class FakeSelects: - def config_setting_group(self, **kwargs): - pass - - -num_cc_libraries = 0 -num_opted_out_cc_libraries = 0 -parsing_path = None - - -# Convert the source or header target to a relative path. -def _get_filename(name, parsing_path): - filename = "%s%s" % ( - ( - parsing_path + "/" - if (parsing_path and not name.startswith("//")) - else "" - ), - name, - ) - filename = filename.replace("//:", "") - filename = filename.replace("//src/core:", "src/core/") - filename = filename.replace( - "//src/cpp/ext/filters/census:", "src/cpp/ext/filters/census/" - ) - return filename - - -def grpc_cc_library( - name, - hdrs=[], - public_hdrs=[], - srcs=[], - select_deps=None, - tags=[], - deps=[], - external_deps=[], - proto=None, - **kwargs, -): - global args - global num_cc_libraries - global num_opted_out_cc_libraries - global parsing_path - assert parsing_path is not None - name = "//%s:%s" % (parsing_path, name) - num_cc_libraries += 1 - if select_deps or "nofixdeps" in tags: - if args.whats_left and not select_deps and "nofixdeps" not in tags: - num_opted_out_cc_libraries += 1 - print("Not opted in: {}".format(name)) - no_update.add(name) - scores[name] = len(public_hdrs + hdrs) - # avoid_dep is the internal way of saying prefer something else - # we add grpc_avoid_dep to allow internal grpc-only stuff to avoid each - # other, whilst not biasing dependent projects - if "avoid_dep" in tags or "grpc_avoid_dep" in tags: - avoidness[name] += 10 - if proto: - proto_hdr = "%s%s" % ( - (parsing_path + "/" if parsing_path else ""), - proto.replace(".proto", ".pb.h"), - ) - skip_headers[name].add(proto_hdr) - - for hdr in hdrs + public_hdrs: - vendors[_get_filename(hdr, parsing_path)].append(name) - inc = set() - original_deps[name] = frozenset(deps) - original_external_deps[name] = frozenset(external_deps) - for src in hdrs + public_hdrs + srcs: - for line in open(_get_filename(src, parsing_path)): - m = re.search(r"^#include <(.*)>", line) - if m: - inc.add(m.group(1)) - m = re.search(r'^#include "(.*)"', line) - if m: - inc.add(m.group(1)) - consumes[name] = list(inc) - - -def grpc_proto_library(name, srcs, **kwargs): - global parsing_path - assert parsing_path is not None - name = "//%s:%s" % (parsing_path, name) - for src in srcs: - proto_hdr = src.replace(".proto", ".pb.h") - vendors[_get_filename(proto_hdr, parsing_path)].append(name) - - -def buildozer(cmd, target): - buildozer_commands.append("%s|%s" % (cmd, target)) - - -def buildozer_set_list(name, values, target, via=""): - if not values: - buildozer("remove %s" % name, target) - return - adjust = via if via else name - buildozer( - "set %s %s" % (adjust, " ".join('"%s"' % s for s in values)), target - ) - if via: - buildozer("remove %s" % name, target) - buildozer("rename %s %s" % (via, name), target) - - -def score_edit_distance(proposed, existing): - """Score a proposed change primarily by edit distance""" - sum = 0 - for p in proposed: - if p not in existing: - sum += 1 - for e in existing: - if e not in proposed: - sum += 1 - return sum - - -def total_score(proposal): - return sum(scores[dep] for dep in proposal) - - -def total_avoidness(proposal): - return sum(avoidness[dep] for dep in proposal) - - -def score_list_size(proposed, existing): - """Score a proposed change primarily by number of dependencies""" - return len(proposed) - - -def score_best(proposed, existing): - """Score a proposed change primarily by dependency score""" - return 0 - - -SCORERS = { - "edit_distance": score_edit_distance, - "list_size": score_list_size, - "best": score_best, -} - -parser = argparse.ArgumentParser(description="Fix build dependencies") -parser.add_argument( - "targets", nargs="*", default=[], help="targets to fix (empty => all)" -) -parser.add_argument( - "--score", - type=str, - default="edit_distance", - help="scoring function to use: one of " + ", ".join(SCORERS.keys()), -) -parser.add_argument( - "--whats_left", - action="store_true", - default=False, - help="show what is left to opt in", -) -parser.add_argument( - "--explain", - action="store_true", - default=False, - help="try to explain some decisions", -) -parser.add_argument( - "--why", - type=str, - default=None, - help="with --explain, target why a given dependency is needed", -) -args = parser.parse_args() - -for dirname in [ - "", - "src/core", - "src/cpp/ext/gcp", - "src/cpp/ext/csm", - "src/cpp/ext/otel", - "test/core/backoff", - "test/core/experiments", - "test/core/uri", - "test/core/util", - "test/core/end2end", - "test/core/event_engine", - "test/core/filters", - "test/core/promise", - "test/core/resource_quota", - "test/core/transport/chaotic_good", - "fuzztest", - "fuzztest/core/channel", - "fuzztest/core/transport/chttp2", -]: - parsing_path = dirname - exec( - open("%sBUILD" % (dirname + "/" if dirname else ""), "r").read(), - { - "load": lambda filename, *args: None, - "licenses": lambda licenses: None, - "package": lambda **kwargs: None, - "exports_files": lambda files, visibility=None: None, - "bool_flag": lambda **kwargs: None, - "config_setting": lambda **kwargs: None, - "selects": FakeSelects(), - "python_config_settings": lambda **kwargs: None, - "grpc_cc_binary": grpc_cc_library, - "grpc_cc_library": grpc_cc_library, - "grpc_cc_test": grpc_cc_library, - "grpc_core_end2end_test": lambda **kwargs: None, - "grpc_fuzzer": grpc_cc_library, - "grpc_fuzz_test": grpc_cc_library, - "grpc_proto_fuzzer": grpc_cc_library, - "grpc_proto_library": grpc_proto_library, - "select": lambda d: d["//conditions:default"], - "glob": lambda files: None, - "grpc_end2end_tests": lambda: None, - "grpc_upb_proto_library": lambda name, **kwargs: None, - "grpc_upb_proto_reflection_library": lambda name, **kwargs: None, - "grpc_generate_one_off_targets": lambda: None, - "grpc_generate_one_off_internal_targets": lambda: None, - "grpc_package": lambda **kwargs: None, - "filegroup": lambda name, **kwargs: None, - "sh_library": lambda name, **kwargs: None, - }, - {}, - ) - parsing_path = None - -if args.whats_left: - print( - "{}/{} libraries are opted in".format( - num_cc_libraries - num_opted_out_cc_libraries, num_cc_libraries - ) - ) - - -def make_relative_path(dep, lib): - if lib is None: - return dep - lib_path = lib[: lib.rfind(":") + 1] - if dep.startswith(lib_path): - return dep[len(lib_path) :] - return dep - - -if args.whats_left: - print( - "{}/{} libraries are opted in".format( - num_cc_libraries - num_opted_out_cc_libraries, num_cc_libraries - ) - ) - - -# Keeps track of all possible sets of dependencies that could satify the -# problem. (models the list monad in Haskell!) -class Choices: - def __init__(self, library, substitutions): - self.library = library - self.to_add = [] - self.to_remove = [] - self.substitutions = substitutions - - def add_one_of(self, choices, trigger): - if not choices: - return - choices = sum( - [self.apply_substitutions(choice) for choice in choices], [] - ) - if args.explain and (args.why is None or args.why in choices): - print( - "{}: Adding one of {} for {}".format( - self.library, choices, trigger - ) - ) - self.to_add.append( - tuple( - make_relative_path(choice, self.library) for choice in choices - ) - ) - - def add(self, choice, trigger): - self.add_one_of([choice], trigger) - - def remove(self, remove): - for remove in self.apply_substitutions(remove): - self.to_remove.append(make_relative_path(remove, self.library)) - - def apply_substitutions(self, dep): - if dep in self.substitutions: - return self.substitutions[dep] - return [dep] - - def best(self, scorer): - choices = set() - choices.add(frozenset()) - - for add in sorted(set(self.to_add), key=lambda x: (len(x), x)): - new_choices = set() - for append_choice in add: - for choice in choices: - new_choices.add(choice.union([append_choice])) - choices = new_choices - for remove in sorted(set(self.to_remove)): - new_choices = set() - for choice in choices: - new_choices.add(choice.difference([remove])) - choices = new_choices - - best = None - - def final_scorer(x): - return (total_avoidness(x), scorer(x), total_score(x)) - - for choice in choices: - if best is None or final_scorer(choice) < final_scorer(best): - best = choice - return best - - -def make_library(library): - error = False - hdrs = sorted(consumes[library]) - # we need a little trickery here since grpc_base has channel.cc, which calls grpc_init - # which is in grpc, which is illegal but hard to change - # once EventEngine lands we can clean this up - deps = Choices( - library, - {"//:grpc_base": ["//:grpc", "//:grpc_unsecure"]} - if library.startswith("//test/") - else {}, - ) - external_deps = Choices(None, {}) - for hdr in hdrs: - if hdr in skip_headers[library]: - continue - - if hdr == "systemd/sd-daemon.h": - continue - - if hdr == "src/core/lib/profiling/stap_probes.h": - continue - - if hdr.startswith("src/libfuzzer/"): - continue - - if hdr == "grpc/grpc.h" and library.startswith("//test:"): - # not the root build including grpc.h ==> //:grpc - deps.add_one_of(["//:grpc", "//:grpc_unsecure"], hdr) - continue - - if hdr in INTERNAL_DEPS: - dep = INTERNAL_DEPS[hdr] - if isinstance(dep, list): - for d in dep: - deps.add(d, hdr) - else: - if not ("//" in dep): - dep = "//:" + dep - deps.add(dep, hdr) - continue - - if hdr in vendors: - deps.add_one_of(vendors[hdr], hdr) - continue - - if "include/" + hdr in vendors: - deps.add_one_of(vendors["include/" + hdr], hdr) - continue - - if "." not in hdr: - # assume a c++ system include - continue - - if hdr in EXTERNAL_DEPS: - if isinstance(EXTERNAL_DEPS[hdr], list): - for dep in EXTERNAL_DEPS[hdr]: - external_deps.add(dep, hdr) - else: - external_deps.add(EXTERNAL_DEPS[hdr], hdr) - continue - - if hdr.startswith("opencensus/"): - trail = hdr[len("opencensus/") :] - trail = trail[: trail.find("/")] - external_deps.add("opencensus-" + trail, hdr) - continue - - if hdr.startswith("envoy/"): - path, file = os.path.split(hdr) - file = file.split(".") - path = path.split("/") - dep = "_".join(path[:-1] + [file[1]]) - deps.add(dep, hdr) - continue - - if hdr.startswith("google/protobuf/") and not hdr.endswith(".upb.h"): - external_deps.add("protobuf_headers", hdr) - continue - - if "/" not in hdr: - # assume a system include - continue - - is_sys_include = False - for sys_path in [ - "sys", - "arpa", - "gperftools", - "netinet", - "linux", - "android", - "mach", - "net", - "CoreFoundation", - ]: - if hdr.startswith(sys_path + "/"): - is_sys_include = True - break - if is_sys_include: - # assume a system include - continue - - print( - "# ERROR: can't categorize header: %s used by %s" % (hdr, library) - ) - error = True - - deps.remove(library) - - deps = sorted( - deps.best(lambda x: SCORERS[args.score](x, original_deps[library])) - ) - external_deps = sorted( - external_deps.best( - lambda x: SCORERS[args.score](x, original_external_deps[library]) - ) - ) - - return (library, error, deps, external_deps) - - -def main() -> None: - update_libraries = [] - for library in sorted(consumes.keys()): - if library in no_update: - continue - if args.targets and library not in args.targets: - continue - update_libraries.append(library) - with multiprocessing.Pool(processes=multiprocessing.cpu_count()) as p: - updated_libraries = p.map(make_library, update_libraries, 1) - - error = False - for library, lib_error, deps, external_deps in updated_libraries: - if lib_error: - error = True - continue - buildozer_set_list("external_deps", external_deps, library, via="deps") - buildozer_set_list("deps", deps, library) - - run_buildozer.run_buildozer(buildozer_commands) - - if error: - sys.exit(1) - - -if __name__ == "__main__": - main() diff --git a/tools/distrib/iwyu.sh b/tools/distrib/iwyu.sh deleted file mode 100755 index 9a00fde2b4d..00000000000 --- a/tools/distrib/iwyu.sh +++ /dev/null @@ -1,63 +0,0 @@ -#!/bin/bash -# Copyright 2021 gRPC authors. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -echo "NOTE: to automagically apply fixes, invoke with --fix" - -set -ex - -# change to root directory -cd $(dirname $0)/../.. -REPO_ROOT=$(pwd) - -# grep targets with manual tag, which is not included in a result of bazel build using ... -# let's get a list of them using query command and pass it to gen_compilation_database.py -export MANUAL_TARGETS=$(bazel query 'attr("tags", "manual", tests(//test/cpp/...))' | grep -v _on_ios) - -# generate a clang compilation database for all C/C++ sources in the repo. -tools/distrib/gen_compilation_database.py \ - --include_headers \ - --ignore_system_headers \ - --dedup_targets \ - "//:*" \ - "//src/core/..." \ - "//src/cpp/ext/csm/..." \ - "//src/cpp/ext/gcp/..." \ - "//src/cpp/ext/otel/..." \ - "//src/compiler/..." \ - "//test/core/..." \ - "//test/cpp/..." \ - "//fuzztest/..." \ - $MANUAL_TARGETS - -# run iwyu against the checked out codebase -# when modifying the checked-out files, the current user will be impersonated -# so that the updated files don't end up being owned by "root". -if [ "$IWYU_SKIP_DOCKER" == "" ] -then - # build iwyu docker image - docker build -t grpc_iwyu tools/dockerfile/grpc_iwyu - - docker run \ - -e TEST="$TEST" \ - -e CHANGED_FILES="$CHANGED_FILES" \ - -e IWYU_ROOT="/local-code" \ - --rm=true \ - -v "${REPO_ROOT}":/local-code \ - -v "${HOME/.cache/bazel}":"${HOME/.cache/bazel}" \ - --user "$(id -u):$(id -g)" \ - -t grpc_iwyu /iwyu.sh "$@" -else - IWYU_ROOT="${REPO_ROOT}" tools/dockerfile/grpc_iwyu/iwyu.sh -fi diff --git a/tools/distrib/iwyu_mappings.imp b/tools/distrib/iwyu_mappings.imp deleted file mode 100644 index 3f993a9aee5..00000000000 --- a/tools/distrib/iwyu_mappings.imp +++ /dev/null @@ -1,9 +0,0 @@ -[ - { include: ["", "private", "", "public"] }, - { include: ["", "public", "\"src/core/lib/iomgr/sockaddr.h\"", "public"]}, - { include: ["", "public", "\"src/core/lib/iomgr/sockaddr.h\"", "public"]}, - { include: ["", "private", "", "public"] }, - { include: ["", "private", "", "public"] }, - # workaround: https://github.com/include-what-you-use/include-what-you-use/issues/908 - { symbol: ["std::max", "private", "", "public" ] }, -] diff --git a/tools/dockerfile/grpc_iwyu/Dockerfile b/tools/dockerfile/grpc_iwyu/Dockerfile deleted file mode 100644 index 01b9a2d3a4a..00000000000 --- a/tools/dockerfile/grpc_iwyu/Dockerfile +++ /dev/null @@ -1,28 +0,0 @@ -# Copyright 2021 gRPC authors. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -# Docker file for running IWYU. -# Updated: 2022-11-03 - -FROM silkeh/clang:16-bullseye - -# Install prerequisites for the iwyu script -RUN apt-get update && apt-get install -y python3 jq git cmake python zlib1g-dev libtinfo-dev libclang-16-dev && apt-get clean -ADD iwyu.sh / - -# When running locally, we'll be impersonating the current user, so we need -# to make the script runnable by everyone. -RUN chmod a+rx /iwyu.sh - -CMD ["echo 'Run with tools/distrib/iwyu.sh'"] diff --git a/tools/dockerfile/grpc_iwyu/iwyu.sh b/tools/dockerfile/grpc_iwyu/iwyu.sh deleted file mode 100755 index 96f5ba74592..00000000000 --- a/tools/dockerfile/grpc_iwyu/iwyu.sh +++ /dev/null @@ -1,139 +0,0 @@ -#!/bin/sh -# Copyright 2017 gRPC authors. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -set -x - -cd ${IWYU_ROOT} - -export PATH=${PATH}:${IWYU_ROOT}/iwyu_build/bin - -# number of CPUs available -CPU_COUNT=`nproc` - -rm -rf iwyu || true -git clone https://github.com/include-what-you-use/include-what-you-use.git iwyu - -############################################################################### -# -# BEWARE! BEWARE! BEWARE! BEWARE! BEWARE! BEWARE! BEWARE! BEWARE! -# -# Changing the version of iwyu can bring along subtle changes. -# You *must* test the new version of iwyu: -# 1. run it on the entire codebase before submitting -# 2. UPLOAD A CHANGE THAT SHOULD BE BROKEN AFTER SUBMISSION OF THIS CHANGE -# ensure that the broken change is caught by the new version of iwyu -# -# BEWARE! BEWARE! BEWARE! BEWARE! BEWARE! BEWARE! BEWARE! BEWARE! -# -############################################################################### - -# latest commit on the clang_16 branch -cd ${IWYU_ROOT}/iwyu -git checkout 7301b1fc88e5e16d8df73aecea55037d9c0a371b -if [ $? -ne 0 ]; then - echo "Failed to checkout iwyu commit" - exit 1 -fi -mkdir -p ${IWYU_ROOT}/iwyu_build -cd ${IWYU_ROOT}/iwyu_build -cmake -G "Unix Makefiles" -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DLLVM_ROOT_DIR=/usr/lib/llvm-16 ${IWYU_ROOT}/iwyu -if [ $? -ne 0 ]; then - echo "Failed to cmake iwyu" - exit 1 -fi -make -j $CPU_COUNT -if [ $? -ne 0 ]; then - echo "Failed to make iwyu" - exit 1 -fi -cd ${IWYU_ROOT} - -cat compile_commands.json \ - | sed "s/ -DNDEBUG//g" \ - | sed "s/ -std=c\\+\\+14/ -std=c++17/g" \ - | sed "s,\"file\": \",\"file\": \"${IWYU_ROOT}/,g" \ - > compile_commands_for_iwyu.json - -export ENABLED_MODULES=' - src/core/ext - src/core/lib - src/cpp - src/python/grpcio_observability - test/core - fuzztest -' - -export DISABLED_MODULES=' - src/core/lib/gpr - src/core/lib/iomgr - src/core/ext/transport/binder - test/core/alts - test/core/iomgr - test/core/security - test/core/tsi - test/core/transport/binder -' - -export INCLUSION_REGEX=`echo $ENABLED_MODULES | sed 's/ /|/g' | sed 's,\\(.*\\),^(\\1)/,g'` -export EXCLUSION_REGEX=`echo $DISABLED_MODULES | sed 's/ /|/g' | sed 's,\\(.*\\),^(\\1)/,g'` - -# figure out which files to include -cat compile_commands.json | jq -r '.[].file' \ - | grep -E $INCLUSION_REGEX \ - | grep -v -E "/upb-gen/|/upbdefs-gen/" \ - | grep -v -E $EXCLUSION_REGEX \ - | grep -v src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h \ - | grep -v test/core/end2end/end2end_tests.cc \ - | sort \ - > iwyu_files0.txt - -cat iwyu_files0.txt \ - | xargs -d '\n' ls -1df 2> /dev/null \ - > iwyu_files.txt \ - || true - -echo '#!/bin/sh -${IWYU_ROOT}/iwyu/iwyu_tool.py -p compile_commands_for_iwyu.json $1 \ - -- -Xiwyu --no_fwd_decls \ - -Xiwyu --update_comments \ - -Xiwyu --mapping_file=${IWYU_ROOT}/tools/distrib/iwyu_mappings.imp \ - | grep -v -E "port_platform.h" \ - | grep -v -E "repeated_ptr_field.h" \ - | grep -v -E "repeated_field.h" \ - | grep -v -E "^(- )?namespace " \ - > iwyu/iwyu.`echo $1 | sha1sum`.out -' > iwyu/run_iwyu_on.sh -chmod +x iwyu/run_iwyu_on.sh - -# run iwyu, filtering out changes to port_platform.h -xargs -n 1 -P $CPU_COUNT -a iwyu_files.txt ${IWYU_ROOT}/iwyu/run_iwyu_on.sh - -cat iwyu/iwyu.*.out > iwyu.out - -# apply the suggested changes -${IWYU_ROOT}/iwyu/fix_includes.py \ - --nocomments \ - --nosafe_headers \ - --ignore_re='^(include/.*|src/core/lib/security/credentials/tls/grpc_tls_credentials_options\.h|external/.*)' \ - < iwyu.out \ - | grep 'IWYU edited 0 files on your behalf' - -if [ $? -ne 0 ] -then - echo "Iwyu edited some files. Here is the diff of files edited by iwyu:" - git --no-pager diff - # Exit with a non zero error code to ensure sanity checks fail accordingly. - exit 1 -fi diff --git a/tools/internal_ci/linux/grpc_iwyu.cfg b/tools/internal_ci/linux/grpc_iwyu.cfg deleted file mode 100644 index 33fbbd68bad..00000000000 --- a/tools/internal_ci/linux/grpc_iwyu.cfg +++ /dev/null @@ -1,30 +0,0 @@ -# Copyright 2017 gRPC authors. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -# Config file for the internal CI (in protobuf text format) - -# Location of the continuous shell script in repository. -build_file: "grpc/tools/internal_ci/linux/grpc_run_tests_matrix.sh" -timeout_mins: 60 -action { - define_artifacts { - regex: "**/*sponge_log.*" - regex: "github/grpc/reports/**" - } -} - -env_vars { - key: "RUN_TESTS_FLAGS" - value: "-f basictests linux iwyu --inner_jobs 16 -j 1 --internal_ci --bq_result_table aggregate_results" -} diff --git a/tools/internal_ci/linux/pull_request/grpc_iwyu.cfg b/tools/internal_ci/linux/pull_request/grpc_iwyu.cfg deleted file mode 100644 index 4d7fcecc9a6..00000000000 --- a/tools/internal_ci/linux/pull_request/grpc_iwyu.cfg +++ /dev/null @@ -1,30 +0,0 @@ -# Copyright 2017 gRPC authors. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -# Config file for the internal CI (in protobuf text format) - -# Location of the continuous shell script in repository. -build_file: "grpc/tools/internal_ci/linux/grpc_run_tests_matrix.sh" -timeout_mins: 60 -action { - define_artifacts { - regex: "**/*sponge_log.*" - regex: "github/grpc/reports/**" - } -} - -env_vars { - key: "RUN_TESTS_FLAGS" - value: "-f basictests linux iwyu --inner_jobs 16 -j 1 --internal_ci" -} diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 64d2cfcbeb0..88a9a648bbd 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -1304,7 +1304,6 @@ _LANGUAGES = { "objc": ObjCLanguage(), "sanity": Sanity("sanity_tests.yaml"), "clang-tidy": Sanity("clang_tidy_tests.yaml"), - "iwyu": Sanity("iwyu_tests.yaml"), } _MSBUILD_CONFIG = { diff --git a/tools/run_tests/sanity/iwyu_tests.yaml b/tools/run_tests/sanity/iwyu_tests.yaml deleted file mode 100644 index 2c9c7836d7f..00000000000 --- a/tools/run_tests/sanity/iwyu_tests.yaml +++ /dev/null @@ -1,2 +0,0 @@ -- script: tools/distrib/iwyu.sh - cpu_cost: 1000 From 92eb194140aff1bde3e045e1c61bf43faf899202 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Wed, 29 Nov 2023 15:27:30 -0800 Subject: [PATCH 04/20] [interop] Add grpc-java 1.59.1 to client_matrix.py (#35130) Closes #35130 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35130 from ejona86:java-1.59.1 8e966842255acaf7a683f8095c0ef5284b87de73 PiperOrigin-RevId: 586470793 --- tools/interop_matrix/client_matrix.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/interop_matrix/client_matrix.py b/tools/interop_matrix/client_matrix.py index 110e29ce2f1..836485bdb69 100644 --- a/tools/interop_matrix/client_matrix.py +++ b/tools/interop_matrix/client_matrix.py @@ -430,7 +430,7 @@ LANG_RELEASE_MATRIX = { ("v1.56.0", ReleaseInfo()), ("v1.57.2", ReleaseInfo()), ("v1.58.0", ReleaseInfo()), - ("v1.59.0", ReleaseInfo()), + ("v1.59.1", ReleaseInfo()), ] ), "python": OrderedDict( From a215eb671775057bfaa55c16d62a9bf3f0b4a1fd Mon Sep 17 00:00:00 2001 From: AJ Heller Date: Wed, 29 Nov 2023 15:29:34 -0800 Subject: [PATCH 05/20] [test] Sleep between server restarts in retry_transparent_max_concurrent_streams (#35149) This hack temporarily quiets the flaky test report for a known race. This is the only end2end test that shuts down & restarts a server in the same test execution. The PosixEventEngine's Listener implementation asynchronously shuts down listening ports after Listener destruction. Some changes can possibly be made here to only proceed in server restart after the `on_shutdown` callback is called, ensuring all ports are closed before proceeding. Closes #35149 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35149 from drfloob:hack/max_concurrent_fix_for_posix_ee_listener 9a7b7b53ddda421d627a7a49458974af852f8c89 PiperOrigin-RevId: 586471281 --- .../end2end/tests/retry_transparent_max_concurrent_streams.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/core/end2end/tests/retry_transparent_max_concurrent_streams.cc b/test/core/end2end/tests/retry_transparent_max_concurrent_streams.cc index ceb37404247..663fd6f6105 100644 --- a/test/core/end2end/tests/retry_transparent_max_concurrent_streams.cc +++ b/test/core/end2end/tests/retry_transparent_max_concurrent_streams.cc @@ -104,6 +104,8 @@ CORE_END2END_TEST(RetryHttp2Test, RetryTransparentMaxConcurrentStreams) { EXPECT_EQ(server_status.status(), GRPC_STATUS_OK); EXPECT_EQ(server_status.message(), "xyz"); // Destroy server and then restart it. + // TODO(hork): hack to solve PosixEventEngine Listener's async shutdown issue. + absl::SleepFor(absl::Milliseconds(250)); InitServer(server_args); // Server should get the second call. auto s2 = RequestCall(201); From 0c78392570c4eb050e8cfeaef9588fcf865e2ac3 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 30 Nov 2023 09:12:37 -0800 Subject: [PATCH 06/20] [autofix] attempt to log actor string (#35182) --- .github/workflows/pr-auto-fix.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/pr-auto-fix.yaml b/.github/workflows/pr-auto-fix.yaml index 2374a5e0e0d..4b930563dca 100644 --- a/.github/workflows/pr-auto-fix.yaml +++ b/.github/workflows/pr-auto-fix.yaml @@ -50,6 +50,7 @@ jobs: // If you'd like not to run this code on your commits, add your github user id here: NO_AUTOFIX_USERS = ["copybara-service"] const { owner, repo } = context.repo + console.log("Actor: " + context.actor); if (NO_AUTOFIX_USERS.includes(context.actor)) { console.log('Cancelling'); const run_id = "${{ github.run_id }}"; From f356ef47e69edf14cc5465cd7763abba68ce4e33 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 30 Nov 2023 10:06:29 -0800 Subject: [PATCH 07/20] [experiments] extend expiration of happy eyeballs experiment (#35181) Closes #35181 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35181 from markdroth:happy_eyeballs_experiment_expiration 5da6d05f610ce0cf303a851f6b517757bca0cff9 PiperOrigin-RevId: 586709581 --- src/core/lib/experiments/experiments.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/lib/experiments/experiments.yaml b/src/core/lib/experiments/experiments.yaml index 60ed3d7eb2e..76273515fdf 100644 --- a/src/core/lib/experiments/experiments.yaml +++ b/src/core/lib/experiments/experiments.yaml @@ -185,7 +185,7 @@ - name: pick_first_happy_eyeballs description: Use Happy Eyeballs in pick_first. - expiry: 2023/12/15 + expiry: 2024/01/15 owner: roth@google.com test_tags: ["lb_unit_test", "cpp_lb_end2end_test", "xds_end2end_test"] - name: ping_on_rst_stream From f9e56a9467eb637bfcc50396712a7c4b80ea748d Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 30 Nov 2023 11:52:35 -0800 Subject: [PATCH 08/20] [TLS certificate provider] remove pollset_set from cert provider API (#35013) We have no cert providers today that actually use iomgr polling, so this API is not actually used anywhere. And even if we wanted to add a cert provider that did use iomgr polling, I don't think it would work correctly, because we are plumbing the pollset_set linkage only for XdsCredentials, not for normal TlsCredentials. Closes #35013 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35013 from markdroth:cert_provider_remove_pollset_set 383cd02ffb6405a77006a97601bb73606e254067 PiperOrigin-RevId: 586743891 --- .../client_channel/lb_policy/xds/cds.cc | 57 +++++-------------- src/core/ext/xds/certificate_provider_store.h | 5 -- .../tls/grpc_tls_certificate_provider.h | 3 - 3 files changed, 14 insertions(+), 51 deletions(-) diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc b/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc index 7e42338d70d..ea42f22e7c0 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc @@ -199,21 +199,18 @@ class CdsLb : public LoadBalancingPolicy { // The root of the tree is config_->cluster(). std::map watchers_; - // TODO(roth, yashkt): These are here because we need to handle - // pollset_set linkage as clusters are added or removed from the - // XdsCertificateProvider. However, in the aggregate cluster case, - // there may be multiple clusters in the same cert provider, and we're - // only tracking the cert providers for the most recent underlying - // cluster here. I think this is a bug that could cause us to starve - // the underlying cert providers of polling. However, it is not - // actually causing any problem in practice today, because (a) we have - // no cert provider impl that relies on gRPC's polling and (b) - // probably no one is actually configuring an aggregate cluster with - // different cert providers in different underlying clusters. - // Hopefully, this problem won't be an issue in practice until after - // the EventEngine migration is done, at which point the need for - // handling pollset_set linkage will go away, and these fields can - // simply be removed. + // TODO(roth, yashkt): These are here because XdsCertificateProvider + // does not store the actual underlying cert providers, it stores only + // their distributors, so we need to hold a ref to the cert providers + // here. However, in the aggregate cluster case, there may be multiple + // clusters in the same cert provider, and we're only tracking the cert + // providers for the most recent underlying cluster here. This is + // clearly a bug, and I think it will cause us to stop getting updates + // for all but one of the cert providers in the aggregate cluster + // case. Need to figure out the right way to fix this -- I don't + // think we want to store another map here, so ideally, we should just + // have XdsCertificateProvider actually hold the refs to the cert + // providers instead of just the distributors. RefCountedPtr root_certificate_provider_; RefCountedPtr identity_certificate_provider_; @@ -610,20 +607,7 @@ absl::Status CdsLb::UpdateXdsCertificateProvider( root_provider_instance_name, "\" not recognized.")); } } - if (root_certificate_provider_ != new_root_provider) { - if (root_certificate_provider_ != nullptr && - root_certificate_provider_->interested_parties() != nullptr) { - grpc_pollset_set_del_pollset_set( - interested_parties(), - root_certificate_provider_->interested_parties()); - } - if (new_root_provider != nullptr && - new_root_provider->interested_parties() != nullptr) { - grpc_pollset_set_add_pollset_set(interested_parties(), - new_root_provider->interested_parties()); - } - root_certificate_provider_ = std::move(new_root_provider); - } + root_certificate_provider_ = std::move(new_root_provider); xds_certificate_provider_->UpdateRootCertNameAndDistributor( cluster_name, root_provider_cert_name, root_certificate_provider_ == nullptr @@ -647,20 +631,7 @@ absl::Status CdsLb::UpdateXdsCertificateProvider( identity_provider_instance_name, "\" not recognized.")); } } - if (identity_certificate_provider_ != new_identity_provider) { - if (identity_certificate_provider_ != nullptr && - identity_certificate_provider_->interested_parties() != nullptr) { - grpc_pollset_set_del_pollset_set( - interested_parties(), - identity_certificate_provider_->interested_parties()); - } - if (new_identity_provider != nullptr && - new_identity_provider->interested_parties() != nullptr) { - grpc_pollset_set_add_pollset_set( - interested_parties(), new_identity_provider->interested_parties()); - } - identity_certificate_provider_ = std::move(new_identity_provider); - } + identity_certificate_provider_ = std::move(new_identity_provider); xds_certificate_provider_->UpdateIdentityCertNameAndDistributor( cluster_name, identity_provider_cert_name, identity_certificate_provider_ == nullptr diff --git a/src/core/ext/xds/certificate_provider_store.h b/src/core/ext/xds/certificate_provider_store.h index 24b172ac32d..aba287f9789 100644 --- a/src/core/ext/xds/certificate_provider_store.h +++ b/src/core/ext/xds/certificate_provider_store.h @@ -36,7 +36,6 @@ #include "src/core/lib/gprpp/sync.h" #include "src/core/lib/gprpp/unique_type_name.h" #include "src/core/lib/gprpp/validation_errors.h" -#include "src/core/lib/iomgr/iomgr_fwd.h" #include "src/core/lib/json/json.h" #include "src/core/lib/json/json_args.h" #include "src/core/lib/json/json_object_loader.h" @@ -96,10 +95,6 @@ class CertificateProviderStore return certificate_provider_->distributor(); } - grpc_pollset_set* interested_parties() const override { - return certificate_provider_->interested_parties(); - } - int CompareImpl(const grpc_tls_certificate_provider* other) const override { // TODO(yashykt): This should probably delegate to the `Compare` method of // the wrapped certificate_provider_ object. diff --git a/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h b/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h index 22ccf7ebd74..d9f2527a33c 100644 --- a/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h +++ b/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h @@ -39,7 +39,6 @@ #include "src/core/lib/gprpp/sync.h" #include "src/core/lib/gprpp/thd.h" #include "src/core/lib/gprpp/unique_type_name.h" -#include "src/core/lib/iomgr/iomgr_fwd.h" #include "src/core/lib/security/credentials/tls/grpc_tls_certificate_distributor.h" #include "src/core/lib/security/security_connector/ssl_utils.h" @@ -55,8 +54,6 @@ struct grpc_tls_certificate_provider : public grpc_core::RefCounted { public: - virtual grpc_pollset_set* interested_parties() const { return nullptr; } - virtual grpc_core::RefCountedPtr distributor() const = 0; From be4d2a6d8b3fa79839bbe5bc43788da046840d05 Mon Sep 17 00:00:00 2001 From: AJ Heller Date: Thu, 30 Nov 2023 13:16:10 -0800 Subject: [PATCH 09/20] [core] Ensure ChannelArgs::SetObject only allows conforming shared_ptr classes (#35008) ChannelArgs shared_ptr only supports types that extend `enable_shared_from_this`. `args.SetObject>(x)` with a non-comforming type X will now fail with something like: ``` ./src/core/lib/channel/channel_args.h:453:12: error: no matching member function for call to 'Set' return Set(ChannelArgNameTraits::ChannelArgName(), std::move(p)); ^~~ test/core/channel/channel_args_test.cc:352:32: note: in instantiation of function template specialization 'grpc_core::ChannelArgs::SetObject' requested here grpc_core::ChannelArgs b = a.SetObject(x); ^ .. ``` Closes #35008 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35008 from drfloob:channel-args-cant-set-unsupported-shared-ptr-type dc93f27ac728fbed233c48aa07fdb839c57ebf0a PiperOrigin-RevId: 586766674 --- src/core/lib/channel/channel_args.h | 26 ++++++++++++------- .../fuzzing_event_engine.h | 4 ++- .../resolver_fuzzer.cc | 3 ++- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/core/lib/channel/channel_args.h b/src/core/lib/channel/channel_args.h index 38bb070213c..78848750294 100644 --- a/src/core/lib/channel/channel_args.h +++ b/src/core/lib/channel/channel_args.h @@ -125,6 +125,16 @@ struct ChannelArgTypeTraits< }; }; +// Define a check for shared_ptr supported types, which must extend +// enable_shared_from_this. +template +struct SupportedSharedPtrType + : std::integral_constant< + bool, std::is_base_of, T>::value> {}; +template <> +struct SupportedSharedPtrType + : std::true_type {}; + // Specialization for shared_ptr // Incurs an allocation because shared_ptr.release is not a thing. template @@ -174,18 +184,12 @@ struct ChannelArgTypeTraits -struct WrapInSharedPtr - : std::integral_constant< - bool, std::is_base_of, T>::value> {}; -template <> -struct WrapInSharedPtr - : std::true_type {}; template struct GetObjectImpl; // std::shared_ptr implementation template -struct GetObjectImpl::value, void>> { +struct GetObjectImpl< + T, absl::enable_if_t::value, void>> { using Result = T*; using ReffedResult = std::shared_ptr; using StoredType = std::shared_ptr*; @@ -205,7 +209,8 @@ struct GetObjectImpl::value, void>> { }; // RefCountedPtr template -struct GetObjectImpl::value, void>> { +struct GetObjectImpl< + T, absl::enable_if_t::value, void>> { using Result = T*; using ReffedResult = RefCountedPtr; using StoredType = Result; @@ -391,6 +396,9 @@ class ChannelArgs { decltype(ChannelArgTypeTraits>::VTable())>::value, ChannelArgs> Set(absl::string_view name, std::shared_ptr value) const { + static_assert(SupportedSharedPtrType::value, + "Type T must extend std::enable_shared_from_this to be added " + "into ChannelArgs as a shared_ptr"); auto* store_value = new std::shared_ptr(value); return Set( name, diff --git a/test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.h b/test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.h index 5f83f44c9ea..78e36a65d2a 100644 --- a/test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.h +++ b/test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.h @@ -48,7 +48,9 @@ namespace experimental { // EventEngine implementation to be used by fuzzers. // It's only allowed to have one FuzzingEventEngine instantiated at a time. -class FuzzingEventEngine : public EventEngine { +class FuzzingEventEngine + : public EventEngine, + public std::enable_shared_from_this { public: struct Options { Duration max_delay_run_after = std::chrono::seconds(30); diff --git a/test/core/ext/filters/event_engine_client_channel_resolver/resolver_fuzzer.cc b/test/core/ext/filters/event_engine_client_channel_resolver/resolver_fuzzer.cc index 483ad6b0e4a..97ed6eaa71c 100644 --- a/test/core/ext/filters/event_engine_client_channel_resolver/resolver_fuzzer.cc +++ b/test/core/ext/filters/event_engine_client_channel_resolver/resolver_fuzzer.cc @@ -71,7 +71,8 @@ absl::Status ErrorToAbslStatus( } class FuzzingResolverEventEngine - : public grpc_event_engine::experimental::AbortingEventEngine { + : public grpc_event_engine::experimental::AbortingEventEngine, + public std::enable_shared_from_this { public: explicit FuzzingResolverEventEngine( const event_engine_client_channel_resolver::Msg& msg, From c10ae5fc636778064dd9bd6f259c1511c7a38e41 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 30 Nov 2023 14:01:46 -0800 Subject: [PATCH 10/20] [autofix] fix copybara username (#35184) --- .github/workflows/pr-auto-fix.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr-auto-fix.yaml b/.github/workflows/pr-auto-fix.yaml index 4b930563dca..654f943c1d4 100644 --- a/.github/workflows/pr-auto-fix.yaml +++ b/.github/workflows/pr-auto-fix.yaml @@ -48,7 +48,7 @@ jobs: with: script: | // If you'd like not to run this code on your commits, add your github user id here: - NO_AUTOFIX_USERS = ["copybara-service"] + NO_AUTOFIX_USERS = ["copybara-service[bot]"] const { owner, repo } = context.repo console.log("Actor: " + context.actor); if (NO_AUTOFIX_USERS.includes(context.actor)) { From ff4058a738d6ea78ebf9bb4f540ca655bf1bcc90 Mon Sep 17 00:00:00 2001 From: Alisha Nanda Date: Thu, 30 Nov 2023 15:01:09 -0800 Subject: [PATCH 11/20] Decrease number of inlined elements in slice_buffer from 8 to 7. PiperOrigin-RevId: 586797742 --- include/grpc/impl/slice_type.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/grpc/impl/slice_type.h b/include/grpc/impl/slice_type.h index 5d618740553..7d9dfb78123 100644 --- a/include/grpc/impl/slice_type.h +++ b/include/grpc/impl/slice_type.h @@ -74,7 +74,7 @@ struct grpc_slice { } data; }; -#define GRPC_SLICE_BUFFER_INLINE_ELEMENTS 8 +#define GRPC_SLICE_BUFFER_INLINE_ELEMENTS 7 /** Represents an expandable array of slices, to be interpreted as a single item. */ From e348d65bc7ce47986c725009b8db47b1e606d878 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 30 Nov 2023 15:24:08 -0800 Subject: [PATCH 12/20] [promises] Add some niceties for StatusFlag, ValueOrFailure PiperOrigin-RevId: 586804010 --- src/core/lib/promise/status_flag.h | 38 ++++++++++++++++++++++++--- test/core/promise/status_flag_test.cc | 2 ++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/core/lib/promise/status_flag.h b/src/core/lib/promise/status_flag.h index 063cb76a079..54019d38740 100644 --- a/src/core/lib/promise/status_flag.h +++ b/src/core/lib/promise/status_flag.h @@ -25,11 +25,31 @@ namespace grpc_core { +struct Failure {}; +struct Success {}; + +inline bool IsStatusOk(Failure) { return false; } +inline bool IsStatusOk(Success) { return true; } + +template <> +struct StatusCastImpl { + static absl::Status Cast(Success) { return absl::OkStatus(); } +}; + +template <> +struct StatusCastImpl { + static absl::Status Cast(Success) { return absl::OkStatus(); } +}; + // A boolean representing whether an operation succeeded (true) or failed // (false). class StatusFlag { public: explicit StatusFlag(bool value) : value_(value) {} + // NOLINTNEXTLINE(google-explicit-constructor) + StatusFlag(Failure) : value_(false) {} + // NOLINTNEXTLINE(google-explicit-constructor) + StatusFlag(Success) : value_(true) {} bool ok() const { return value_; } @@ -46,14 +66,21 @@ struct StatusCastImpl { } }; -struct Failure {}; +template <> +struct StatusCastImpl { + static absl::Status Cast(StatusFlag flag) { + return flag.ok() ? absl::OkStatus() : absl::CancelledError(); + } +}; // A value if an operation was successful, or a failure flag if not. template class ValueOrFailure { public: - explicit ValueOrFailure(T value) : value_(std::move(value)) {} - explicit ValueOrFailure(Failure) {} + // NOLINTNEXTLINE(google-explicit-constructor) + ValueOrFailure(T value) : value_(std::move(value)) {} + // NOLINTNEXTLINE(google-explicit-constructor) + ValueOrFailure(Failure) {} static ValueOrFailure FromOptional(absl::optional value) { return ValueOrFailure{std::move(value)}; @@ -75,6 +102,11 @@ inline bool IsStatusOk(const ValueOrFailure& value) { return value.ok(); } +template +inline T TakeValue(ValueOrFailure&& value) { + return std::move(value.value()); +} + template struct StatusCastImpl> { static absl::Status Cast(const ValueOrFailure flag) { diff --git a/test/core/promise/status_flag_test.cc b/test/core/promise/status_flag_test.cc index 799a08917e7..dcde3641f04 100644 --- a/test/core/promise/status_flag_test.cc +++ b/test/core/promise/status_flag_test.cc @@ -33,6 +33,8 @@ TEST(StatusFlagTest, Basics) { EXPECT_EQ(ValueOrFailure(42).value(), 42); EXPECT_EQ(StatusCast>(ValueOrFailure(42)).value(), 42); + EXPECT_TRUE(IsStatusOk(Success{})); + EXPECT_FALSE(IsStatusOk(Failure{})); } } // namespace grpc_core From 6e18346c96e145a94493a2a2fceb4f1c46d04b18 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Thu, 30 Nov 2023 16:08:13 -0800 Subject: [PATCH 13/20] [otel] Update OTel to head (#35151) This is mainly to get the fix made in https://github.com/open-telemetry/opentelemetry-cpp/pull/2213 When opentelemetry-cpp makes a stable release with this fix, we'll switch to that. Closes #35151 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35151 from yashykt:UpdateOTel 1041fbc0bce2d7091607eb671fb8bad91af79fcc PiperOrigin-RevId: 586815878 --- bazel/grpc_deps.bzl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bazel/grpc_deps.bzl b/bazel/grpc_deps.bzl index a982d8079b5..1f64e9fe5b4 100644 --- a/bazel/grpc_deps.bzl +++ b/bazel/grpc_deps.bzl @@ -512,11 +512,11 @@ def grpc_deps(): if "io_opentelemetry_cpp" not in native.existing_rules(): http_archive( name = "io_opentelemetry_cpp", - sha256 = "f30cd88bf898a5726d245eba882b8e81012021eb00df34109f4dfb203f005cea", - strip_prefix = "opentelemetry-cpp-1.11.0", + sha256 = "149f076cc7a79bbd3a3c34fb3ab61d3a3e8dcfe2b9596f79153e17123c32f897", + strip_prefix = "opentelemetry-cpp-064fef0d871c57ffac6739d3311659a5770a9db4", urls = [ - "https://storage.googleapis.com/grpc-bazel-mirror/github.com/open-telemetry/opentelemetry-cpp/archive/refs/tags/v1.11.0.tar.gz", - "https://github.com/open-telemetry/opentelemetry-cpp/archive/refs/tags/v1.11.0.tar.gz", + "https://storage.googleapis.com/grpc-bazel-mirror/github.com/open-telemetry/opentelemetry-cpp/archive/064fef0d871c57ffac6739d3311659a5770a9db4.tar.gz", + "https://github.com/open-telemetry/opentelemetry-cpp/archive/064fef0d871c57ffac6739d3311659a5770a9db4.tar.gz", ], ) From e481f6acc5c1bf560eea5095dbbf36e7fae77fd5 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 1 Dec 2023 09:05:25 -0800 Subject: [PATCH 14/20] [promises] Generate a better error message for a common mistake (#35191) Closes #35191 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35191 from ctiller:cg-promise-like-fix 0683ffe16943bee355dd97735b972a7315aabb93 PiperOrigin-RevId: 587026178 --- src/core/lib/promise/detail/promise_like.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/core/lib/promise/detail/promise_like.h b/src/core/lib/promise/detail/promise_like.h index 486653856e3..4bec3661642 100644 --- a/src/core/lib/promise/detail/promise_like.h +++ b/src/core/lib/promise/detail/promise_like.h @@ -68,6 +68,10 @@ class PromiseLike { private: GPR_NO_UNIQUE_ADDRESS F f_; + static_assert(!std::is_void::type>::value, + "PromiseLike cannot be used with a function that returns void " + "- return Empty{} instead"); + public: // NOLINTNEXTLINE - internal detail that drastically simplifies calling code. PromiseLike(F&& f) : f_(std::forward(f)) {} From 49f7ee96d195f44e21d5b57106f2564b6e101b15 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 1 Dec 2023 11:06:38 -0800 Subject: [PATCH 15/20] [promises] Convert http-client filter to v3 filter API (#35189) Also start building a temporary wrapping layer so that the new style filters can execute as promise filters directly. Closes #35189 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35189 from ctiller:cg-http-cli dbd73e8aca651b3c597d78e47b4d52e173e7b02b PiperOrigin-RevId: 587060881 --- .../filters/http/client/http_client_filter.cc | 54 ++-- .../filters/http/client/http_client_filter.h | 14 +- src/core/lib/channel/promise_based_filter.h | 285 ++++++++++++++++++ 3 files changed, 316 insertions(+), 37 deletions(-) diff --git a/src/core/ext/filters/http/client/http_client_filter.cc b/src/core/ext/filters/http/client/http_client_filter.cc index 8b547bac6ee..07fcba62581 100644 --- a/src/core/ext/filters/http/client/http_client_filter.cc +++ b/src/core/ext/filters/http/client/http_client_filter.cc @@ -51,6 +51,9 @@ namespace grpc_core { +const NoInterceptor HttpClientFilter::Call::OnServerToClientMessage; +const NoInterceptor HttpClientFilter::Call::OnClientToServerMessage; + const grpc_channel_filter HttpClientFilter::kFilter = MakePromiseBasedFilter("http-client"); @@ -105,40 +108,27 @@ Slice UserAgentFromArgs(const ChannelArgs& args, } } // namespace -ArenaPromise HttpClientFilter::MakeCallPromise( - CallArgs call_args, NextPromiseFactory next_promise_factory) { - auto& md = call_args.client_initial_metadata; - if (test_only_use_put_requests_) { - md->Set(HttpMethodMetadata(), HttpMethodMetadata::kPut); +void HttpClientFilter::Call::OnClientInitialMetadata(ClientMetadata& md, + HttpClientFilter* filter) { + if (filter->test_only_use_put_requests_) { + md.Set(HttpMethodMetadata(), HttpMethodMetadata::kPut); } else { - md->Set(HttpMethodMetadata(), HttpMethodMetadata::kPost); + md.Set(HttpMethodMetadata(), HttpMethodMetadata::kPost); } - md->Set(HttpSchemeMetadata(), scheme_); - md->Set(TeMetadata(), TeMetadata::kTrailers); - md->Set(ContentTypeMetadata(), ContentTypeMetadata::kApplicationGrpc); - md->Set(UserAgentMetadata(), user_agent_.Ref()); - - auto* initial_metadata_err = - GetContext()->New>(); - - call_args.server_initial_metadata->InterceptAndMap( - [initial_metadata_err]( - ServerMetadataHandle md) -> absl::optional { - auto r = CheckServerMetadata(md.get()); - if (!r.ok()) { - initial_metadata_err->Set(ServerMetadataFromStatus(r)); - return absl::nullopt; - } - return std::move(md); - }); - - return Race(initial_metadata_err->Wait(), - Map(next_promise_factory(std::move(call_args)), - [](ServerMetadataHandle md) -> ServerMetadataHandle { - auto r = CheckServerMetadata(md.get()); - if (!r.ok()) return ServerMetadataFromStatus(r); - return md; - })); + md.Set(HttpSchemeMetadata(), filter->scheme_); + md.Set(TeMetadata(), TeMetadata::kTrailers); + md.Set(ContentTypeMetadata(), ContentTypeMetadata::kApplicationGrpc); + md.Set(UserAgentMetadata(), filter->user_agent_.Ref()); +} + +absl::Status HttpClientFilter::Call::OnServerInitialMetadata( + ServerMetadata& md) { + return CheckServerMetadata(&md); +} + +absl::Status HttpClientFilter::Call::OnServerTrailingMetadata( + ServerMetadata& md) { + return CheckServerMetadata(&md); } HttpClientFilter::HttpClientFilter(HttpSchemeMetadata::ValueType scheme, diff --git a/src/core/ext/filters/http/client/http_client_filter.h b/src/core/ext/filters/http/client/http_client_filter.h index 298daf03c67..3146ea07385 100644 --- a/src/core/ext/filters/http/client/http_client_filter.h +++ b/src/core/ext/filters/http/client/http_client_filter.h @@ -25,23 +25,27 @@ #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/channel_fwd.h" #include "src/core/lib/channel/promise_based_filter.h" -#include "src/core/lib/promise/arena_promise.h" #include "src/core/lib/slice/slice.h" #include "src/core/lib/transport/metadata_batch.h" #include "src/core/lib/transport/transport.h" namespace grpc_core { -class HttpClientFilter : public ChannelFilter { +class HttpClientFilter : public ImplementChannelFilter { public: static const grpc_channel_filter kFilter; static absl::StatusOr Create( const ChannelArgs& args, ChannelFilter::Args filter_args); - // Construct a promise for one call. - ArenaPromise MakeCallPromise( - CallArgs call_args, NextPromiseFactory next_promise_factory) override; + class Call { + public: + void OnClientInitialMetadata(ClientMetadata& md, HttpClientFilter* filter); + absl::Status OnServerInitialMetadata(ServerMetadata& md); + absl::Status OnServerTrailingMetadata(ServerMetadata& md); + static const NoInterceptor OnClientToServerMessage; + static const NoInterceptor OnServerToClientMessage; + }; private: HttpClientFilter(HttpSchemeMetadata::ValueType scheme, Slice user_agent, diff --git a/src/core/lib/channel/promise_based_filter.h b/src/core/lib/channel/promise_based_filter.h index 19efe505db2..25ee1230ef1 100644 --- a/src/core/lib/channel/promise_based_filter.h +++ b/src/core/lib/channel/promise_based_filter.h @@ -43,6 +43,7 @@ #include #include "src/core/lib/channel/call_finalization.h" +#include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/channel_fwd.h" #include "src/core/lib/channel/channel_stack.h" #include "src/core/lib/channel/context.h" @@ -60,6 +61,7 @@ #include "src/core/lib/promise/context.h" #include "src/core/lib/promise/pipe.h" #include "src/core/lib/promise/poll.h" +#include "src/core/lib/promise/race.h" #include "src/core/lib/resource_quota/arena.h" #include "src/core/lib/slice/slice_buffer.h" #include "src/core/lib/surface/call.h" @@ -122,6 +124,289 @@ class ChannelFilter { grpc_event_engine::experimental::GetDefaultEventEngine(); }; +struct NoInterceptor {}; + +namespace promise_filter_detail { + +// Determine if a list of interceptors has any that need to asyncronously error +// the promise. If so, we need to allocate a latch for the generated promise for +// the original promise stack polyfill code that's generated. + +inline constexpr bool HasAsyncErrorInterceptor() { return false; } + +inline constexpr bool HasAsyncErrorInterceptor(const NoInterceptor*) { + return false; +} + +template +inline constexpr bool HasAsyncErrorInterceptor(absl::Status (T::*)(A...)) { + return true; +} + +template +inline constexpr bool HasAsyncErrorInterceptor(void (T::*)(A...)) { + return false; +} + +// For the list case we do two interceptors to avoid amiguities with the single +// argument forms above. +template +inline constexpr bool HasAsyncErrorInterceptor(I1 i1, I2 i2, + Interceptors... interceptors) { + return HasAsyncErrorInterceptor(i1) || HasAsyncErrorInterceptor(i2) || + HasAsyncErrorInterceptor(interceptors...); +} + +// Composite for a given channel type to determine if any of its interceptors +// fall into this category: later code should use this. +template +inline constexpr bool CallHasAsyncErrorInterceptor() { + return HasAsyncErrorInterceptor(&Derived::Call::OnClientToServerMessage, + &Derived::Call::OnServerInitialMetadata, + &Derived::Call::OnServerToClientMessage); +} + +// Determine if an interceptor needs to access the channel via one of its +// arguments. If so, we need to allocate a pointer to the channel for the +// generated polyfill promise for the original promise stack. + +inline constexpr bool HasChannelAccess() { return false; } + +inline constexpr bool HasChannelAccess(const NoInterceptor*) { return false; } + +template +inline constexpr bool HasChannelAccess(R (T::*)(A)) { + return false; +} + +template +inline constexpr bool HasChannelAccess(R (T::*)(A, C)) { + return true; +} + +// For the list case we do two interceptors to avoid amiguities with the single +// argument forms above. +template +inline constexpr bool HasChannelAccess(I1 i1, I2 i2, + Interceptors... interceptors) { + return HasChannelAccess(i1) || HasChannelAccess(i2) || + HasChannelAccess(interceptors...); +} + +// Composite for a given channel type to determine if any of its interceptors +// fall into this category: later code should use this. +template +inline constexpr bool CallHasChannelAccess() { + return HasChannelAccess(&Derived::Call::OnClientInitialMetadata, + &Derived::Call::OnClientToServerMessage, + &Derived::Call::OnServerInitialMetadata, + &Derived::Call::OnServerToClientMessage, + &Derived::Call::OnServerTrailingMetadata); +} + +// Given a boolean X export a type: +// either T if X is true +// or an empty type if it is false +template +struct TypeIfNeeded; + +template +struct TypeIfNeeded { + struct Type { + Type() = default; + template + explicit Type(Whatever) : Type() {} + }; +}; + +template +struct TypeIfNeeded { + using Type = T; +}; + +// For the original promise scheme polyfill: +// If a set of interceptors might fail asynchronously, wrap the main +// promise in a race with the cancellation latch. +// If not, just return the main promise. +template +struct RaceAsyncCompletion; + +template <> +struct RaceAsyncCompletion { + template + static Promise Run(Promise x, void*) { + return x; + } +}; + +template <> +struct RaceAsyncCompletion { + template + static Promise Run(Promise x, Latch* latch) { + return Race(latch->Wait(), std::move(x)); + } +}; + +// For the original promise scheme polyfill: data associated with once call. +template +struct FilterCallData { + explicit FilterCallData(Derived* channel) : channel(channel) {} + GPR_NO_UNIQUE_ADDRESS typename Derived::Call call; + GPR_NO_UNIQUE_ADDRESS + typename TypeIfNeeded, + CallHasAsyncErrorInterceptor()>::Type + error_latch; + GPR_NO_UNIQUE_ADDRESS + typename TypeIfNeeded()>::Type + channel; +}; + +template +auto MapResult(const NoInterceptor*, Promise x, void*) { + return x; +} + +template +auto MapResult(absl::Status (Derived::Call::*fn)(ServerMetadata&), Promise x, + FilterCallData* call_data) { + GPR_DEBUG_ASSERT(fn == &Derived::Call::OnServerTrailingMetadata); + return Map(std::move(x), [call_data](ServerMetadataHandle md) { + auto status = call_data->call.OnServerTrailingMetadata(*md); + if (!status.ok()) return ServerMetadataFromStatus(status); + return md; + }); +} + +inline auto RunCall(const NoInterceptor*, CallArgs call_args, + NextPromiseFactory next_promise_factory, void*) { + return next_promise_factory(std::move(call_args)); +} + +template +inline auto RunCall(void (Derived::Call::*fn)(ClientMetadata& md), + CallArgs call_args, NextPromiseFactory next_promise_factory, + FilterCallData* call_data) { + GPR_DEBUG_ASSERT(fn == &Derived::Call::OnClientInitialMetadata); + call_data->call.OnClientInitialMetadata(*call_args.client_initial_metadata); + return next_promise_factory(std::move(call_args)); +} + +template +inline auto RunCall(void (Derived::Call::*fn)(ClientMetadata& md, + Derived* channel), + CallArgs call_args, NextPromiseFactory next_promise_factory, + FilterCallData* call_data) { + GPR_DEBUG_ASSERT(fn == &Derived::Call::OnClientInitialMetadata); + call_data->call.OnClientInitialMetadata(*call_args.client_initial_metadata, + call_data->channel); + return next_promise_factory(std::move(call_args)); +} + +inline void InterceptClientToServerMessage(const NoInterceptor*, void*, + CallArgs&) {} + +inline void InterceptServerInitialMetadata(const NoInterceptor*, void*, + CallArgs&) {} + +template +inline void InterceptServerInitialMetadata( + absl::Status (Derived::Call::*fn)(ServerMetadata&), + FilterCallData* call_data, CallArgs& call_args) { + GPR_DEBUG_ASSERT(fn == &Derived::Call::OnServerInitialMetadata); + call_args.server_initial_metadata->InterceptAndMap( + [call_data]( + ServerMetadataHandle md) -> absl::optional { + auto status = call_data->call.OnServerInitialMetadata(*md); + if (!status.ok() && !call_data->error_latch.is_set()) { + call_data->error_latch.Set(ServerMetadataFromStatus(status)); + return absl::nullopt; + } + return std::move(md); + }); +} + +inline void InterceptServerToClientMessage(const NoInterceptor*, void*, + CallArgs&) {} + +template +absl::enable_if_t>::value, + FilterCallData*> +MakeFilterCall(Derived*) { + static FilterCallData call{nullptr}; + return &call; +} + +template +absl::enable_if_t>::value, + FilterCallData*> +MakeFilterCall(Derived* derived) { + return GetContext()->ManagedNew>(derived); +} + +} // namespace promise_filter_detail + +// Base class for promise-based channel filters. +// Eventually this machinery will move elsewhere (the interception logic will +// move directly into the channel stack, and so filters will just directly +// derive from `ChannelFilter`) +// +// Implements new-style call filters, and polyfills them into the previous +// scheme. +// +// Call filters: +// Derived types should declare a class `Call` with the following members: +// - OnClientInitialMetadata - $VALUE_TYPE = ClientMetadata +// - OnServerInitialMetadata - $VALUE_TYPE = ServerMetadata +// - OnServerToClientMessage - $VALUE_TYPE = Message +// - OnClientToServerMessage - $VALUE_TYPE = Message +// - OnServerTrailingMetadata - $VALUE_TYPE = ServerMetadata +// These members define an interception point for a particular event in +// the call lifecycle. +// The type of these members matters, and is selectable by the class +// author. For $INTERCEPTOR_NAME in the above list: +// - static const NoInterceptor $INTERCEPTOR_NAME: +// defines that this filter does not intercept this event. +// there is zero runtime cost added to handling that event by this filter. +// - void $INTERCEPTOR_NAME($VALUE_TYPE&): +// the filter intercepts this event, and can modify the value. +// it never fails. +// - absl::Status $INTERCEPTOR_NAME($VALUE_TYPE&): +// the filter intercepts this event, and can modify the value. +// it can fail, in which case the call will be aborted. +// - void $INTERCEPTOR_NAME($VALUE_TYPE&, Derived*): +// the filter intercepts this event, and can modify the value. +// it can access the channel via the second argument. +// it never fails. +// - absl::Status $INTERCEPTOR_NAME($VALUE_TYPE&, Derived*): +// the filter intercepts this event, and can modify the value. +// it can access the channel via the second argument. +// it can fail, in which case the call will be aborted. +template +class ImplementChannelFilter : public ChannelFilter { + public: + ArenaPromise MakeCallPromise( + CallArgs call_args, NextPromiseFactory next_promise_factory) final { + auto* call = promise_filter_detail::MakeFilterCall( + static_cast(this)); + promise_filter_detail::InterceptClientToServerMessage( + &Derived::Call::OnClientToServerMessage, call, call_args); + promise_filter_detail::InterceptServerInitialMetadata( + &Derived::Call::OnServerInitialMetadata, call, call_args); + promise_filter_detail::InterceptServerToClientMessage( + &Derived::Call::OnServerToClientMessage, call, call_args); + return promise_filter_detail::MapResult( + &Derived::Call::OnServerTrailingMetadata, + promise_filter_detail::RaceAsyncCompletion< + promise_filter_detail::CallHasAsyncErrorInterceptor()>:: + Run(promise_filter_detail::RunCall( + &Derived::Call::OnClientInitialMetadata, + std::move(call_args), std::move(next_promise_factory), + call), + &call->error_latch), + call); + } +}; + // Designator for whether a filter is client side or server side. // Please don't use this outside calls to MakePromiseBasedFilter - it's // intended to be deleted once the promise conversion is complete. From 84678829af643474e5a8cd468066be3ef1559fae Mon Sep 17 00:00:00 2001 From: Vignesh Babu Date: Fri, 1 Dec 2023 11:41:36 -0800 Subject: [PATCH 16/20] [EventEngine] Add public methods to allow EventEngine Endpoints to support optional Extensions. PiperOrigin-RevId: 587071965 --- CMakeLists.txt | 35 +++++++ build_autogenerated.yaml | 13 +++ include/grpc/event_engine/event_engine.h | 39 ++++++++ src/core/BUILD | 12 +++ src/core/lib/event_engine/query_extensions.h | 70 ++++++++++++++ .../lib/iomgr/event_engine_shims/endpoint.cc | 13 +++ .../lib/iomgr/event_engine_shims/endpoint.h | 5 + test/core/event_engine/BUILD | 13 +++ .../event_engine/query_extensions_test.cc | 95 +++++++++++++++++++ tools/run_tests/generated/tests.json | 24 +++++ 10 files changed, 319 insertions(+) create mode 100644 src/core/lib/event_engine/query_extensions.h create mode 100644 test/core/event_engine/query_extensions_test.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index 4c8979a56ec..9d2a53f4ed6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1229,6 +1229,7 @@ if(gRPC_BUILD_TESTS) add_dependencies(buildtests_cxx proxy_auth_test) add_dependencies(buildtests_cxx qps_json_driver) add_dependencies(buildtests_cxx qps_worker) + add_dependencies(buildtests_cxx query_extensions_test) add_dependencies(buildtests_cxx race_test) add_dependencies(buildtests_cxx random_early_detection_test) add_dependencies(buildtests_cxx raw_end2end_test) @@ -18938,6 +18939,40 @@ target_link_libraries(qps_worker ) +endif() +if(gRPC_BUILD_TESTS) + +add_executable(query_extensions_test + test/core/event_engine/query_extensions_test.cc +) +target_compile_features(query_extensions_test PUBLIC cxx_std_14) +target_include_directories(query_extensions_test + PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR} + ${CMAKE_CURRENT_SOURCE_DIR}/include + ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} + ${_gRPC_RE2_INCLUDE_DIR} + ${_gRPC_SSL_INCLUDE_DIR} + ${_gRPC_UPB_GENERATED_DIR} + ${_gRPC_UPB_GRPC_GENERATED_DIR} + ${_gRPC_UPB_INCLUDE_DIR} + ${_gRPC_XXHASH_INCLUDE_DIR} + ${_gRPC_ZLIB_INCLUDE_DIR} + third_party/googletest/googletest/include + third_party/googletest/googletest + third_party/googletest/googlemock/include + third_party/googletest/googlemock + ${_gRPC_PROTO_GENS_DIR} +) + +target_link_libraries(query_extensions_test + ${_gRPC_ALLTARGETS_LIBRARIES} + gtest + absl::statusor + gpr +) + + endif() if(gRPC_BUILD_TESTS) diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index bd2a9aa0bbd..7eba4bf9d53 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -13309,6 +13309,19 @@ targets: deps: - grpc++_test_config - grpc++_test_util +- name: query_extensions_test + gtest: true + build: test + language: c++ + headers: + - src/core/lib/event_engine/query_extensions.h + src: + - test/core/event_engine/query_extensions_test.cc + deps: + - gtest + - absl/status:statusor + - gpr + uses_polling: false - name: race_test gtest: true build: test diff --git a/include/grpc/event_engine/event_engine.h b/include/grpc/event_engine/event_engine.h index 4beca657625..20cbc64f52f 100644 --- a/include/grpc/event_engine/event_engine.h +++ b/include/grpc/event_engine/event_engine.h @@ -255,6 +255,45 @@ class EventEngine : public std::enable_shared_from_this { /// values are expected to remain valid for the life of the Endpoint. virtual const ResolvedAddress& GetPeerAddress() const = 0; virtual const ResolvedAddress& GetLocalAddress() const = 0; + + /// A method which allows users to query whether an Endpoint implementation + /// supports a specified extension. The name of the extension is provided + /// as an input. + /// + /// An extension could be any type with a unique string id. Each extension + /// may support additional capabilities and if the Endpoint implementation + /// supports the queried extension, it should return a valid pointer to the + /// extension type. + /// + /// E.g., use case of an EventEngine::Endpoint supporting a custom + /// extension. + /// + /// class CustomEndpointExtension { + /// public: + /// static constexpr std::string name = "my.namespace.extension_name"; + /// void Process() { ... } + /// } + /// + /// + /// class CustomEndpoint : + /// public EventEngine::Endpoint, CustomEndpointExtension { + /// public: + /// void* QueryExtension(absl::string_view id) override { + /// if (id == CustomEndpointExtension::name) { + /// return static_cast(this); + /// } + /// return nullptr; + /// } + /// ... + /// } + /// + /// auto ext_ = + /// static_cast( + /// endpoint->QueryExtension(CustomrEndpointExtension::name)); + /// if (ext_ != nullptr) { ext_->Process(); } + /// + /// + virtual void* QueryExtension(absl::string_view /*id*/) { return nullptr; } }; /// Called when a new connection is established. diff --git a/src/core/BUILD b/src/core/BUILD index 351f101b819..e6a4457ac0f 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -1540,6 +1540,18 @@ grpc_cc_library( ], ) +grpc_cc_library( + name = "event_engine_query_extensions", + hdrs = [ + "lib/event_engine/query_extensions.h", + ], + external_deps = ["absl/strings"], + deps = [ + "//:event_engine_base_hdrs", + "//:gpr_platform", + ], +) + grpc_cc_library( name = "event_engine_work_queue", hdrs = [ diff --git a/src/core/lib/event_engine/query_extensions.h b/src/core/lib/event_engine/query_extensions.h new file mode 100644 index 00000000000..2ef15ccfdab --- /dev/null +++ b/src/core/lib/event_engine/query_extensions.h @@ -0,0 +1,70 @@ +// Copyright 2023 gRPC Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#ifndef GRPC_SRC_CORE_LIB_EVENT_ENGINE_QUERY_EXTENSIONS_H +#define GRPC_SRC_CORE_LIB_EVENT_ENGINE_QUERY_EXTENSIONS_H + +#include + +#include "absl/strings/string_view.h" + +#include + +namespace grpc_event_engine { +namespace experimental { + +namespace endpoint_detail { + +template +struct QueryExtensionRecursion; + +template +struct QueryExtensionRecursion { + static void* Query(absl::string_view id, Querying* p) { + if (id == E::EndpointExtensionName()) return static_cast(p); + return QueryExtensionRecursion::Query(id, p); + } +}; + +template +struct QueryExtensionRecursion { + static void* Query(absl::string_view, Querying*) { return nullptr; } +}; + +} // namespace endpoint_detail + +// A helper class to derive from some set of base classes and export +// QueryExtension for them all. +// Endpoint implementations which need to support different extensions just need +// to derive from ExtendedEndpoint class. +template +class ExtendedEndpoint : public EventEngine::Endpoint, public Exports... { + public: + void* QueryExtension(absl::string_view id) override { + return endpoint_detail::QueryExtensionRecursion::Query(id, + this); + } +}; + +/// A helper method which returns a valid pointer if the extension is supported +/// by the endpoint. +template +T* QueryExtension(EventEngine::Endpoint* endpoint) { + return static_cast(endpoint->QueryExtension(T::EndpointExtensionName())); +} + +} // namespace experimental +} // namespace grpc_event_engine + +#endif // GRPC_SRC_CORE_LIB_EVENT_ENGINE_QUERY_EXTENSIONS_H diff --git a/src/core/lib/iomgr/event_engine_shims/endpoint.cc b/src/core/lib/iomgr/event_engine_shims/endpoint.cc index 341fe1e5776..b1e8fdf8904 100644 --- a/src/core/lib/iomgr/event_engine_shims/endpoint.cc +++ b/src/core/lib/iomgr/event_engine_shims/endpoint.cc @@ -69,6 +69,8 @@ class EventEngineEndpointWrapper { explicit EventEngineEndpointWrapper( std::unique_ptr endpoint); + EventEngine::Endpoint* endpoint() { return endpoint_.get(); } + int Fd() { grpc_core::MutexLock lock(&mu_); return fd_; @@ -428,6 +430,17 @@ bool grpc_is_event_engine_endpoint(grpc_endpoint* ep) { return ep->vtable == &grpc_event_engine_endpoint_vtable; } +EventEngine::Endpoint* grpc_get_wrapped_event_engine_endpoint( + grpc_endpoint* ep) { + if (!grpc_is_event_engine_endpoint(ep)) { + return nullptr; + } + auto* eeep = + reinterpret_cast( + ep); + return eeep->wrapper->endpoint(); +} + void grpc_event_engine_endpoint_destroy_and_release_fd( grpc_endpoint* ep, int* fd, grpc_closure* on_release_fd) { auto* eeep = diff --git a/src/core/lib/iomgr/event_engine_shims/endpoint.h b/src/core/lib/iomgr/event_engine_shims/endpoint.h index bc018f1e4d7..efd57c6ea6d 100644 --- a/src/core/lib/iomgr/event_engine_shims/endpoint.h +++ b/src/core/lib/iomgr/event_engine_shims/endpoint.h @@ -31,6 +31,11 @@ grpc_endpoint* grpc_event_engine_endpoint_create( /// Returns true if the passed endpoint is an event engine shim endpoint. bool grpc_is_event_engine_endpoint(grpc_endpoint* ep); +/// Returns the wrapped event engine endpoint if the given grpc_endpoint is an +/// event engine shim endpoint. Otherwise it returns nullptr. +EventEngine::Endpoint* grpc_get_wrapped_event_engine_endpoint( + grpc_endpoint* ep); + /// Destroys the passed in event engine shim endpoint and schedules the /// asynchronous execution of the on_release_fd callback. The int pointer fd is /// set to the underlying endpoint's file descriptor. diff --git a/test/core/event_engine/BUILD b/test/core/event_engine/BUILD index 13543244c14..1cdf7d24cd3 100644 --- a/test/core/event_engine/BUILD +++ b/test/core/event_engine/BUILD @@ -232,3 +232,16 @@ grpc_cc_library( "//src/core:time", ], ) + +grpc_cc_test( + name = "query_extensions_test", + srcs = ["query_extensions_test.cc"], + external_deps = ["gtest"], + language = "C++", + uses_event_engine = False, + uses_polling = False, + deps = [ + "//:gpr_platform", + "//src/core:event_engine_query_extensions", + ], +) diff --git a/test/core/event_engine/query_extensions_test.cc b/test/core/event_engine/query_extensions_test.cc new file mode 100644 index 00000000000..712a496f38c --- /dev/null +++ b/test/core/event_engine/query_extensions_test.cc @@ -0,0 +1,95 @@ +// Copyright 2023 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#include + +#include "src/core/lib/event_engine/query_extensions.h" + +#include + +#include "absl/functional/any_invocable.h" +#include "absl/status/status.h" +#include "gtest/gtest.h" + +#include +#include + +#include "src/core/lib/gprpp/crash.h" + +namespace grpc_event_engine { +namespace experimental { +namespace { + +template +class TestExtension { + public: + TestExtension() = default; + ~TestExtension() = default; + + static std::string EndpointExtensionName() { + return "grpc.test.test_extension" + std::to_string(i); + } + + int GetValue() const { return val_; } + + private: + int val_ = i; +}; + +class ExtendedTestEndpoint + : public ExtendedEndpoint, TestExtension<1>, + TestExtension<2>> { + public: + ExtendedTestEndpoint() = default; + ~ExtendedTestEndpoint() override = default; + bool Read(absl::AnyInvocable /*on_read*/, + SliceBuffer* /*buffer*/, const ReadArgs* /*args*/) override { + grpc_core::Crash("Not implemented"); + }; + bool Write(absl::AnyInvocable /*on_writable*/, + SliceBuffer* /*data*/, const WriteArgs* /*args*/) override { + grpc_core::Crash("Not implemented"); + } + /// Returns an address in the format described in DNSResolver. The returned + /// values are expected to remain valid for the life of the Endpoint. + const EventEngine::ResolvedAddress& GetPeerAddress() const override { + grpc_core::Crash("Not implemented"); + } + const EventEngine::ResolvedAddress& GetLocalAddress() const override { + grpc_core::Crash("Not implemented"); + }; +}; + +TEST(QueryExtensionsTest, EndpointSupportsMultipleExtensions) { + ExtendedTestEndpoint endpoint; + TestExtension<0>* extension_0 = QueryExtension>(&endpoint); + TestExtension<1>* extension_1 = QueryExtension>(&endpoint); + TestExtension<2>* extension_2 = QueryExtension>(&endpoint); + + EXPECT_NE(extension_0, nullptr); + EXPECT_NE(extension_1, nullptr); + EXPECT_NE(extension_2, nullptr); + + EXPECT_EQ(extension_0->GetValue(), 0); + EXPECT_EQ(extension_1->GetValue(), 1); + EXPECT_EQ(extension_2->GetValue(), 2); +} +} // namespace + +} // namespace experimental +} // namespace grpc_event_engine + +int main(int argc, char** argv) { + testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 35bdddc0eb0..d58a72accec 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -7189,6 +7189,30 @@ ], "uses_polling": true }, + { + "args": [], + "benchmark": false, + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "gtest": true, + "language": "c++", + "name": "query_extensions_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "uses_polling": false + }, { "args": [], "benchmark": false, From 1d4ecf66298234578e5a9781df42b72866db7924 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Fri, 1 Dec 2023 12:13:37 -0800 Subject: [PATCH 17/20] [RefCounted] allow RefCounted<> to work for const types (#35188) Closes #35188 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35188 from markdroth:ref_counted_const e2dc753b6b234c74e8374e860f2946c840d3b45c PiperOrigin-RevId: 587081377 --- src/core/lib/gprpp/ref_counted.h | 47 ++++++++++++++++++++++------- test/core/gprpp/ref_counted_test.cc | 11 +++++++ 2 files changed, 47 insertions(+), 11 deletions(-) diff --git a/src/core/lib/gprpp/ref_counted.h b/src/core/lib/gprpp/ref_counted.h index cdf692c5ce7..5eaf5cda0f1 100644 --- a/src/core/lib/gprpp/ref_counted.h +++ b/src/core/lib/gprpp/ref_counted.h @@ -219,7 +219,7 @@ class NonPolymorphicRefCount { // Default behavior: Delete the object. struct UnrefDelete { template - void operator()(T* p) { + void operator()(T* p) const { delete p; } }; @@ -231,7 +231,7 @@ struct UnrefDelete { // later by identifying entries for which RefIfNonZero() returns null. struct UnrefNoDelete { template - void operator()(T* /*p*/) {} + void operator()(T* /*p*/) const {} }; // Call the object's dtor but do not delete it. This is useful for cases @@ -239,7 +239,7 @@ struct UnrefNoDelete { // arena). struct UnrefCallDtor { template - void operator()(T* p) { + void operator()(T* p) const { p->~T(); } }; @@ -279,32 +279,44 @@ class RefCounted : public Impl { // Note: Depending on the Impl used, this dtor can be implicitly virtual. ~RefCounted() = default; + // Ref() for mutable types. GRPC_MUST_USE_RESULT RefCountedPtr Ref() { IncrementRefCount(); return RefCountedPtr(static_cast(this)); } - GRPC_MUST_USE_RESULT RefCountedPtr Ref(const DebugLocation& location, const char* reason) { IncrementRefCount(location, reason); return RefCountedPtr(static_cast(this)); } + // Ref() for const types. + GRPC_MUST_USE_RESULT RefCountedPtr Ref() const { + IncrementRefCount(); + return RefCountedPtr(static_cast(this)); + } + GRPC_MUST_USE_RESULT RefCountedPtr Ref( + const DebugLocation& location, const char* reason) const { + IncrementRefCount(location, reason); + return RefCountedPtr(static_cast(this)); + } + // TODO(roth): Once all of our code is converted to C++ and can use // RefCountedPtr<> instead of manual ref-counting, make this method // private, since it will only be used by RefCountedPtr<>, which is a // friend of this class. - void Unref() { + void Unref() const { if (GPR_UNLIKELY(refs_.Unref())) { - unref_behavior_(static_cast(this)); + unref_behavior_(static_cast(this)); } } - void Unref(const DebugLocation& location, const char* reason) { + void Unref(const DebugLocation& location, const char* reason) const { if (GPR_UNLIKELY(refs_.Unref(location, reason))) { - unref_behavior_(static_cast(this)); + unref_behavior_(static_cast(this)); } } + // RefIfNonZero() for mutable types. GRPC_MUST_USE_RESULT RefCountedPtr RefIfNonZero() { return RefCountedPtr(refs_.RefIfNonZero() ? static_cast(this) : nullptr); @@ -316,6 +328,18 @@ class RefCounted : public Impl { : nullptr); } + // RefIfNonZero() for const types. + GRPC_MUST_USE_RESULT RefCountedPtr RefIfNonZero() const { + return RefCountedPtr( + refs_.RefIfNonZero() ? static_cast(this) : nullptr); + } + GRPC_MUST_USE_RESULT RefCountedPtr RefIfNonZero( + const DebugLocation& location, const char* reason) const { + return RefCountedPtr(refs_.RefIfNonZero(location, reason) + ? static_cast(this) + : nullptr); + } + // Not copyable nor movable. RefCounted(const RefCounted&) = delete; RefCounted& operator=(const RefCounted&) = delete; @@ -336,12 +360,13 @@ class RefCounted : public Impl { template friend class RefCountedPtr; - void IncrementRefCount() { refs_.Ref(); } - void IncrementRefCount(const DebugLocation& location, const char* reason) { + void IncrementRefCount() const { refs_.Ref(); } + void IncrementRefCount(const DebugLocation& location, + const char* reason) const { refs_.Ref(location, reason); } - RefCount refs_; + mutable RefCount refs_; GPR_NO_UNIQUE_ADDRESS UnrefBehavior unref_behavior_; }; diff --git a/test/core/gprpp/ref_counted_test.cc b/test/core/gprpp/ref_counted_test.cc index 4d8761ecb1d..7c28cddc6f6 100644 --- a/test/core/gprpp/ref_counted_test.cc +++ b/test/core/gprpp/ref_counted_test.cc @@ -53,6 +53,17 @@ TEST(RefCounted, ExtraRef) { foo->Unref(); } +TEST(RefCounted, Const) { + const Foo* foo = new Foo(); + RefCountedPtr foop = foo->Ref(); + foop.release(); + foop = foo->RefIfNonZero(); + foop.release(); + foo->Unref(); + foo->Unref(); + foo->Unref(); +} + class Value : public RefCounted { public: Value(int value, std::set>* registry) : value_(value) { From 39493f93c06e0adb46dea88a095f4115fb79d0c1 Mon Sep 17 00:00:00 2001 From: Esun Kim Date: Fri, 1 Dec 2023 16:23:11 -0800 Subject: [PATCH 18/20] Making windows/dll test no-op temporarily. This will be reenabled once DLL work is done. PiperOrigin-RevId: 587155376 --- test/distrib/cpp/run_distrib_test_cmake_for_dll.bat | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/distrib/cpp/run_distrib_test_cmake_for_dll.bat b/test/distrib/cpp/run_distrib_test_cmake_for_dll.bat index 2adfaf14141..887c20dcd74 100644 --- a/test/distrib/cpp/run_distrib_test_cmake_for_dll.bat +++ b/test/distrib/cpp/run_distrib_test_cmake_for_dll.bat @@ -78,6 +78,11 @@ popd @rem folders, like the following command trying to imitate. git submodule foreach bash -c "cd $toplevel; rm -rf $name" +@rem TODO(dawidcha): Remove this once this DLL test can pass { +echo Skipped! +exit /b 0 +@rem TODO(dawidcha): Remove this once this DLL test can pass } + @rem Install gRPC @rem NOTE(jtattermusch): The -DProtobuf_USE_STATIC_LIBS=ON is necessary on cmake3.16+ @rem since by default "find_package(Protobuf ...)" uses the cmake's builtin From addd18b186855f998c729daf36e8dad49f84dcc7 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 1 Dec 2023 17:52:42 -0800 Subject: [PATCH 19/20] [channel-args] Enforce const-correctness for RefCounted values (#35199) Closes #35199 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35199 from ctiller:refcount a3f856858a31b87c7501806223ee82a8edcb9900 PiperOrigin-RevId: 587178819 --- src/core/lib/channel/channel_args.h | 63 ++++++++++++++++++++++++-- test/core/channel/channel_args_test.cc | 31 +++++++++++++ 2 files changed, 89 insertions(+), 5 deletions(-) diff --git a/src/core/lib/channel/channel_args.h b/src/core/lib/channel/channel_args.h index 78848750294..2c10d955127 100644 --- a/src/core/lib/channel/channel_args.h +++ b/src/core/lib/channel/channel_args.h @@ -183,13 +183,27 @@ struct ChannelArgTypeTraits +struct ChannelArgPointerShouldBeConst { + static constexpr bool kValue = false; +}; + +template +struct ChannelArgPointerShouldBeConst< + T, absl::void_t> { + static constexpr bool kValue = T::ChannelArgUseConstPtr(); +}; + // GetObject support for shared_ptr and RefCountedPtr template struct GetObjectImpl; // std::shared_ptr implementation template struct GetObjectImpl< - T, absl::enable_if_t::value, void>> { + T, absl::enable_if_t::kValue && + SupportedSharedPtrType::value, + void>> { using Result = T*; using ReffedResult = std::shared_ptr; using StoredType = std::shared_ptr*; @@ -210,7 +224,9 @@ struct GetObjectImpl< // RefCountedPtr template struct GetObjectImpl< - T, absl::enable_if_t::value, void>> { + T, absl::enable_if_t::kValue && + !SupportedSharedPtrType::value, + void>> { using Result = T*; using ReffedResult = RefCountedPtr; using StoredType = Result; @@ -226,6 +242,26 @@ struct GetObjectImpl< }; }; +template +struct GetObjectImpl< + T, absl::enable_if_t::kValue && + !SupportedSharedPtrType::value, + void>> { + using Result = const T*; + using ReffedResult = RefCountedPtr; + using StoredType = Result; + static Result Get(StoredType p) { return p; }; + static ReffedResult GetReffed(StoredType p) { + if (p == nullptr) return nullptr; + return p->Ref(); + }; + static ReffedResult GetReffed(StoredType p, const DebugLocation& location, + const char* reason) { + if (p == nullptr) return nullptr; + return p->Ref(location, reason); + }; +}; + // Provide the canonical name for a type's channel arg key template struct ChannelArgNameTraits { @@ -242,6 +278,7 @@ struct ChannelArgNameTraits { return GRPC_INTERNAL_ARG_EVENT_ENGINE; } }; + class ChannelArgs { public: class Pointer { @@ -381,15 +418,29 @@ class ChannelArgs { GRPC_MUST_USE_RESULT auto Set(absl::string_view name, RefCountedPtr value) const -> absl::enable_if_t< - std::is_same>::VTable())>::value, + !ChannelArgPointerShouldBeConst::kValue && + std::is_same>::VTable())>::value, ChannelArgs> { return Set( name, Pointer(value.release(), ChannelArgTypeTraits>::VTable())); } template + GRPC_MUST_USE_RESULT auto Set(absl::string_view name, + RefCountedPtr value) const + -> absl::enable_if_t< + ChannelArgPointerShouldBeConst::kValue && + std::is_same>::VTable())>::value, + ChannelArgs> { + return Set( + name, Pointer(const_cast(value.release()), + ChannelArgTypeTraits>::VTable())); + } + template GRPC_MUST_USE_RESULT absl::enable_if_t< std::is_same< const grpc_arg_pointer_vtable*, @@ -426,6 +477,8 @@ class ChannelArgs { absl::optional GetInt(absl::string_view name) const; absl::optional GetString(absl::string_view name) const; absl::optional GetOwnedString(absl::string_view name) const; + // WARNING: this is broken if `name` represents something that was stored as a + // RefCounted - we will discard the const-ness. void* GetVoidPointer(absl::string_view name) const; template typename GetObjectImpl::StoredType GetPointer( diff --git a/test/core/channel/channel_args_test.cc b/test/core/channel/channel_args_test.cc index 10a05d35e26..fd035ccc12d 100644 --- a/test/core/channel/channel_args_test.cc +++ b/test/core/channel/channel_args_test.cc @@ -209,6 +209,37 @@ TEST(ChannelArgsTest, GetNonOwningEventEngine) { ASSERT_EQ(p.use_count(), 2); } +struct MutableValue : public RefCounted { + static constexpr absl::string_view ChannelArgName() { + return "grpc.test.mutable_value"; + } + static int ChannelArgsCompare(const MutableValue* a, const MutableValue* b) { + return a->i - b->i; + } + int i = 42; +}; + +struct ConstValue : public RefCounted { + static constexpr absl::string_view ChannelArgName() { + return "grpc.test.const_value"; + } + static constexpr bool ChannelArgUseConstPtr() { return true; }; + static int ChannelArgsCompare(const ConstValue* a, const ConstValue* b) { + return a->i - b->i; + } + int i = 42; +}; + +TEST(ChannelArgsTest, SetObjectRespectsMutabilityConstraints) { + auto m = MakeRefCounted(); + auto c = MakeRefCounted(); + auto args = ChannelArgs().SetObject(m).SetObject(c); + RefCountedPtr m1 = args.GetObjectRef(); + RefCountedPtr c1 = args.GetObjectRef(); + EXPECT_EQ(m1.get(), m.get()); + EXPECT_EQ(c1.get(), c.get()); +} + } // namespace grpc_core TEST(GrpcChannelArgsTest, Create) { From 7047cc17a8337f8c841ae1b5a6afe4bc5b8a72c6 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 1 Dec 2023 17:53:23 -0800 Subject: [PATCH 20/20] [promises] Migrate http server filter to new API (#35197) Closes #35197 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35197 from ctiller:cg-http-svr cdde418a81dff4ad85c7196dc3e3464e429bf9cc PiperOrigin-RevId: 587178983 --- .../filters/http/server/http_server_filter.cc | 81 +++++++++---------- .../filters/http/server/http_server_filter.h | 14 +++- src/core/lib/channel/promise_based_filter.h | 65 +++++++++++++++ 3 files changed, 115 insertions(+), 45 deletions(-) diff --git a/src/core/ext/filters/http/server/http_server_filter.cc b/src/core/ext/filters/http/server/http_server_filter.cc index 2d4953dd26b..830b931520f 100644 --- a/src/core/ext/filters/http/server/http_server_filter.cc +++ b/src/core/ext/filters/http/server/http_server_filter.cc @@ -49,6 +49,9 @@ namespace grpc_core { +const NoInterceptor HttpServerFilter::Call::OnClientToServerMessage; +const NoInterceptor HttpServerFilter::Call::OnServerToClientMessage; + const grpc_channel_filter HttpServerFilter::kFilter = MakePromiseBasedFilter("http-server"); @@ -71,85 +74,81 @@ ServerMetadataHandle MalformedRequest(absl::string_view explanation) { } } // namespace -ArenaPromise HttpServerFilter::MakeCallPromise( - CallArgs call_args, NextPromiseFactory next_promise_factory) { - const auto& md = call_args.client_initial_metadata; - - auto method = md->get(HttpMethodMetadata()); +ServerMetadataHandle HttpServerFilter::Call::OnClientInitialMetadata( + ClientMetadata& md, HttpServerFilter* filter) { + auto method = md.get(HttpMethodMetadata()); if (method.has_value()) { switch (*method) { case HttpMethodMetadata::kPost: break; case HttpMethodMetadata::kPut: - if (allow_put_requests_) { + if (filter->allow_put_requests_) { break; } ABSL_FALLTHROUGH_INTENDED; case HttpMethodMetadata::kInvalid: case HttpMethodMetadata::kGet: - return Immediate(MalformedRequest("Bad method header")); + return MalformedRequest("Bad method header"); } } else { - return Immediate(MalformedRequest("Missing :method header")); + return MalformedRequest("Missing :method header"); } - auto te = md->Take(TeMetadata()); + auto te = md.Take(TeMetadata()); if (te == TeMetadata::kTrailers) { // Do nothing, ok. } else if (!te.has_value()) { - return Immediate(MalformedRequest("Missing :te header")); + return MalformedRequest("Missing :te header"); } else { - return Immediate(MalformedRequest("Bad :te header")); + return MalformedRequest("Bad :te header"); } - auto scheme = md->Take(HttpSchemeMetadata()); + auto scheme = md.Take(HttpSchemeMetadata()); if (scheme.has_value()) { if (*scheme == HttpSchemeMetadata::kInvalid) { - return Immediate(MalformedRequest("Bad :scheme header")); + return MalformedRequest("Bad :scheme header"); } } else { - return Immediate(MalformedRequest("Missing :scheme header")); + return MalformedRequest("Missing :scheme header"); } - md->Remove(ContentTypeMetadata()); + md.Remove(ContentTypeMetadata()); - Slice* path_slice = md->get_pointer(HttpPathMetadata()); + Slice* path_slice = md.get_pointer(HttpPathMetadata()); if (path_slice == nullptr) { - return Immediate(MalformedRequest("Missing :path header")); + return MalformedRequest("Missing :path header"); } - if (md->get_pointer(HttpAuthorityMetadata()) == nullptr) { - absl::optional host = md->Take(HostMetadata()); + if (md.get_pointer(HttpAuthorityMetadata()) == nullptr) { + absl::optional host = md.Take(HostMetadata()); if (host.has_value()) { - md->Set(HttpAuthorityMetadata(), std::move(*host)); + md.Set(HttpAuthorityMetadata(), std::move(*host)); } } - if (md->get_pointer(HttpAuthorityMetadata()) == nullptr) { - return Immediate(MalformedRequest("Missing :authority header")); + if (md.get_pointer(HttpAuthorityMetadata()) == nullptr) { + return MalformedRequest("Missing :authority header"); } - if (!surface_user_agent_) { - md->Remove(UserAgentMetadata()); + if (!filter->surface_user_agent_) { + md.Remove(UserAgentMetadata()); } - call_args.server_initial_metadata->InterceptAndMap( - [](ServerMetadataHandle md) { - if (grpc_call_trace.enabled()) { - gpr_log(GPR_INFO, "%s[http-server] Write metadata", - Activity::current()->DebugTag().c_str()); - } - FilterOutgoingMetadata(md.get()); - md->Set(HttpStatusMetadata(), 200); - md->Set(ContentTypeMetadata(), ContentTypeMetadata::kApplicationGrpc); - return md; - }); - - return Map(next_promise_factory(std::move(call_args)), - [](ServerMetadataHandle md) -> ServerMetadataHandle { - FilterOutgoingMetadata(md.get()); - return md; - }); + return nullptr; +} + +void HttpServerFilter::Call::OnServerInitialMetadata(ServerMetadata& md) { + if (grpc_call_trace.enabled()) { + gpr_log(GPR_INFO, "%s[http-server] Write metadata", + Activity::current()->DebugTag().c_str()); + } + FilterOutgoingMetadata(&md); + md.Set(HttpStatusMetadata(), 200); + md.Set(ContentTypeMetadata(), ContentTypeMetadata::kApplicationGrpc); +} + +void HttpServerFilter::Call::OnServerTrailingMetadata(ServerMetadata& md) { + FilterOutgoingMetadata(&md); } absl::StatusOr HttpServerFilter::Create( diff --git a/src/core/ext/filters/http/server/http_server_filter.h b/src/core/ext/filters/http/server/http_server_filter.h index bc97bd53b8a..43eeb148957 100644 --- a/src/core/ext/filters/http/server/http_server_filter.h +++ b/src/core/ext/filters/http/server/http_server_filter.h @@ -32,16 +32,22 @@ namespace grpc_core { // Processes metadata on the server side for HTTP2 transports -class HttpServerFilter : public ChannelFilter { +class HttpServerFilter : public ImplementChannelFilter { public: static const grpc_channel_filter kFilter; static absl::StatusOr Create( const ChannelArgs& args, ChannelFilter::Args filter_args); - // Construct a promise for one call. - ArenaPromise MakeCallPromise( - CallArgs call_args, NextPromiseFactory next_promise_factory) override; + class Call { + public: + ServerMetadataHandle OnClientInitialMetadata(ClientMetadata& md, + HttpServerFilter* filter); + void OnServerInitialMetadata(ServerMetadata& md); + void OnServerTrailingMetadata(ServerMetadata& md); + static const NoInterceptor OnClientToServerMessage; + static const NoInterceptor OnServerToClientMessage; + }; private: HttpServerFilter(bool surface_user_agent, bool allow_put_requests) diff --git a/src/core/lib/channel/promise_based_filter.h b/src/core/lib/channel/promise_based_filter.h index 25ee1230ef1..a94b79bbd0c 100644 --- a/src/core/lib/channel/promise_based_filter.h +++ b/src/core/lib/channel/promise_based_filter.h @@ -61,6 +61,7 @@ #include "src/core/lib/promise/context.h" #include "src/core/lib/promise/pipe.h" #include "src/core/lib/promise/poll.h" +#include "src/core/lib/promise/promise.h" #include "src/core/lib/promise/race.h" #include "src/core/lib/resource_quota/arena.h" #include "src/core/lib/slice/slice_buffer.h" @@ -143,6 +144,12 @@ inline constexpr bool HasAsyncErrorInterceptor(absl::Status (T::*)(A...)) { return true; } +template +inline constexpr bool HasAsyncErrorInterceptor( + ServerMetadataHandle (T::*)(A...)) { + return true; +} + template inline constexpr bool HasAsyncErrorInterceptor(void (T::*)(A...)) { return false; @@ -277,6 +284,16 @@ auto MapResult(absl::Status (Derived::Call::*fn)(ServerMetadata&), Promise x, }); } +template +auto MapResult(void (Derived::Call::*fn)(ServerMetadata&), Promise x, + FilterCallData* call_data) { + GPR_DEBUG_ASSERT(fn == &Derived::Call::OnServerTrailingMetadata); + return Map(std::move(x), [call_data](ServerMetadataHandle md) { + call_data->call.OnServerTrailingMetadata(*md); + return md; + }); +} + inline auto RunCall(const NoInterceptor*, CallArgs call_args, NextPromiseFactory next_promise_factory, void*) { return next_promise_factory(std::move(call_args)); @@ -291,6 +308,31 @@ inline auto RunCall(void (Derived::Call::*fn)(ClientMetadata& md), return next_promise_factory(std::move(call_args)); } +template +inline auto RunCall( + ServerMetadataHandle (Derived::Call::*fn)(ClientMetadata& md), + CallArgs call_args, NextPromiseFactory next_promise_factory, + FilterCallData* call_data) -> ArenaPromise { + GPR_DEBUG_ASSERT(fn == &Derived::Call::OnClientInitialMetadata); + auto return_md = call_data->call.OnClientInitialMetadata( + *call_args.client_initial_metadata); + if (return_md == nullptr) return next_promise_factory(std::move(call_args)); + return Immediate(std::move(return_md)); +} + +template +inline auto RunCall(ServerMetadataHandle (Derived::Call::*fn)( + ClientMetadata& md, Derived* channel), + CallArgs call_args, NextPromiseFactory next_promise_factory, + FilterCallData* call_data) + -> ArenaPromise { + GPR_DEBUG_ASSERT(fn == &Derived::Call::OnClientInitialMetadata); + auto return_md = call_data->call.OnClientInitialMetadata( + *call_args.client_initial_metadata, call_data->channel); + if (return_md == nullptr) return next_promise_factory(std::move(call_args)); + return Immediate(std::move(return_md)); +} + template inline auto RunCall(void (Derived::Call::*fn)(ClientMetadata& md, Derived* channel), @@ -308,6 +350,18 @@ inline void InterceptClientToServerMessage(const NoInterceptor*, void*, inline void InterceptServerInitialMetadata(const NoInterceptor*, void*, CallArgs&) {} +template +inline void InterceptServerInitialMetadata( + void (Derived::Call::*fn)(ServerMetadata&), + FilterCallData* call_data, CallArgs& call_args) { + GPR_DEBUG_ASSERT(fn == &Derived::Call::OnServerInitialMetadata); + call_args.server_initial_metadata->InterceptAndMap( + [call_data](ServerMetadataHandle md) { + call_data->call.OnServerInitialMetadata(*md); + return md; + }); +} + template inline void InterceptServerInitialMetadata( absl::Status (Derived::Call::*fn)(ServerMetadata&), @@ -373,6 +427,11 @@ MakeFilterCall(Derived* derived) { // - absl::Status $INTERCEPTOR_NAME($VALUE_TYPE&): // the filter intercepts this event, and can modify the value. // it can fail, in which case the call will be aborted. +// - ServerMetadataHandle $INTERCEPTOR_NAME($VALUE_TYPE&) +// the filter intercepts this event, and can modify the value. +// the filter can return nullptr for success, or a metadata handle for +// failure (in which case the call will be aborted). +// useful for cases where the exact metadata returned needs to be customized. // - void $INTERCEPTOR_NAME($VALUE_TYPE&, Derived*): // the filter intercepts this event, and can modify the value. // it can access the channel via the second argument. @@ -381,6 +440,12 @@ MakeFilterCall(Derived* derived) { // the filter intercepts this event, and can modify the value. // it can access the channel via the second argument. // it can fail, in which case the call will be aborted. +// - ServerMetadataHandle $INTERCEPTOR_NAME($VALUE_TYPE&, Derived*) +// the filter intercepts this event, and can modify the value. +// it can access the channel via the second argument. +// the filter can return nullptr for success, or a metadata handle for +// failure (in which case the call will be aborted). +// useful for cases where the exact metadata returned needs to be customized. template class ImplementChannelFilter : public ChannelFilter { public: