From e06e45f2af390144a164d50879f16d0bc108e4bc Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Thu, 14 Mar 2024 15:31:01 -0700 Subject: [PATCH] [OTel C++] Do not add otel_plugin_test with CMake if the otel plugin is not enabled (#36128) This should fix our interop builds Closes #36128 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36128 from yashykt:OTelPluginCIFix 98165a868b0b0c9060b179a1636afdb65976f15d PiperOrigin-RevId: 615923868 --- CMakeLists.txt | 3 +-- build_autogenerated.yaml | 3 ++- templates/CMakeLists.txt.template | 8 ++++++- templates/README.md | 1 + tools/buildgen/build_cleaner.py | 1 + .../extract_metadata_from_bazel_xml.py | 17 ++++++++++--- tools/buildgen/plugins/check_attrs.py | 1 + tools/run_tests/generated/tests.json | 24 ------------------- 8 files changed, 27 insertions(+), 31 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index afaffdda098..3ef168cffd9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1238,7 +1238,6 @@ if(gRPC_BUILD_TESTS) add_dependencies(buildtests_cxx orca_service_end2end_test) add_dependencies(buildtests_cxx orphanable_test) add_dependencies(buildtests_cxx osa_distance_test) - add_dependencies(buildtests_cxx otel_plugin_test) add_dependencies(buildtests_cxx out_of_bounds_bad_client_test) add_dependencies(buildtests_cxx outlier_detection_lb_config_parser_test) add_dependencies(buildtests_cxx outlier_detection_test) @@ -20620,7 +20619,7 @@ target_link_libraries(osa_distance_test endif() -if(gRPC_BUILD_TESTS) +if(gRPC_BUILD_TESTS AND gRPC_BUILD_GRPCPP_OTEL_PLUGIN) add_executable(otel_plugin_test ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo.pb.cc diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 8f3765d9398..adb59a57a16 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -13210,8 +13210,9 @@ targets: - gtest - name: otel_plugin_test gtest: true - build: test + build: plugin_test language: c++ + plugin_option: gRPC_BUILD_GRPCPP_OTEL_PLUGIN headers: - src/cpp/ext/otel/key_value_iterable.h - src/cpp/ext/otel/otel_call_tracer.h diff --git a/templates/CMakeLists.txt.template b/templates/CMakeLists.txt.template index 3e69ad92258..ad184e021a1 100644 --- a/templates/CMakeLists.txt.template +++ b/templates/CMakeLists.txt.template @@ -200,7 +200,7 @@ def is_generate_cmake_target(lib_or_target): """Returns True if a cmake target should be generated for given library/target.""" # TODO(jtattermusch): extract the metadata to a centralized location. - if lib_or_target.build not in ["all", "plugin", "protoc", "tool", "test", "private"]: + if lib_or_target.build not in ["all", "plugin", "plugin_test", "protoc", "tool", "test", "private"]: return False if lib_or_target.boringssl: # Don't generate target for boringssl libs or tests @@ -775,6 +775,12 @@ ${cc_binary(tgt)} endif() + % elif tgt.build in ["plugin_test"]: + if(gRPC_BUILD_TESTS AND ${tgt.plugin_option}) + <%block filter='platforms_condition_block(tgt.platforms)'> + ${cc_binary(tgt)} + + endif() % elif tgt.build in ["protoc"]: if(gRPC_BUILD_CODEGEN AND gRPC_BUILD_${tgt.name.upper()}) <%block filter='platforms_condition_block(tgt.platforms)'> diff --git a/templates/README.md b/templates/README.md index 03e5836d133..87685a11ad0 100644 --- a/templates/README.md +++ b/templates/README.md @@ -91,6 +91,7 @@ Currently, the "`build`" tag have these meanings: * `"all"`: library to build on `"make all"`, and install on the system. * `"plugin"`: library to build on `"make all"`, and install, but the corresponding CMake option defaults to off. The option needs to be declared manually at present to allow depending on third-party dependencies. * `"protoc"`: a protoc plugin to build on `"make all"` and install on the system. +* `"plugin_test"`: A test that should only be built if the associated plugin is enabled. The plugin is mentioned in the `"plugin_option"` tag. * `"private"`: a library to only build for tests. * `"test"`: a test binary to run on `"make test"`. * `"tool"`: a binary to be built upon `"make tools"`. diff --git a/tools/buildgen/build_cleaner.py b/tools/buildgen/build_cleaner.py index 6071ec50535..e2546fab85e 100755 --- a/tools/buildgen/build_cleaner.py +++ b/tools/buildgen/build_cleaner.py @@ -39,6 +39,7 @@ _ELEM_KEYS = [ "build", "run", "language", + "plugin_option", "public_headers", "headers", "src", diff --git a/tools/buildgen/extract_metadata_from_bazel_xml.py b/tools/buildgen/extract_metadata_from_bazel_xml.py index bcf57fab2c4..d261c96efb2 100755 --- a/tools/buildgen/extract_metadata_from_bazel_xml.py +++ b/tools/buildgen/extract_metadata_from_bazel_xml.py @@ -536,7 +536,10 @@ def update_test_metadata_with_transitive_metadata( """Patches test build metadata with transitive metadata.""" for lib_name, lib_dict in list(all_extra_metadata.items()): # Skip if it isn't not an test - if lib_dict.get("build") != "test" or lib_dict.get("_TYPE") != "target": + if ( + lib_dict.get("build") != "test" + and lib_dict.get("build") != "plugin_test" + ) or lib_dict.get("_TYPE") != "target": continue bazel_rule = bazel_rules[_get_bazel_label(lib_name)] @@ -752,6 +755,7 @@ def _convert_to_build_yaml_like(lib_dict: BuildMetadata) -> BuildYaml: lib_name for lib_name in list(lib_dict.keys()) if lib_dict[lib_name].get("_TYPE", "library") == "test" + or lib_dict[lib_name].get("_TYPE", "library") == "plugin_test" ] # list libraries and targets in predefined order @@ -1006,7 +1010,7 @@ def _generate_build_extra_metadata_for_tests( " to %s" % (test_name, long_name) ) test_metadata[test_name]["_RENAME"] = long_name - + print(test_metadata["test/cpp/ext/otel:otel_plugin_test"]) return test_metadata @@ -1301,6 +1305,13 @@ _BUILD_EXTRA_METADATA = { "_TYPE": "target", "_RENAME": "grpc_cli", }, + "test/cpp/ext/otel:otel_plugin_test": { + "language": "c++", + "build": "plugin_test", + "_TYPE": "target", + "plugin_option": "gRPC_BUILD_GRPCPP_OTEL_PLUGIN", + "_RENAME": "otel_plugin_test", + }, # TODO(jtattermusch): create_jwt and verify_jwt breaks distribtests because it depends on grpc_test_utils and thus requires tests to be built # For now it's ok to disable them as these binaries aren't very useful anyway. # 'test/core/security:create_jwt': { 'language': 'c', 'build': 'tool', '_TYPE': 'target', '_RENAME': 'grpc_create_jwt' }, @@ -1418,10 +1429,10 @@ if "@com_google_protobuf//third_party/utf8_range:utf8_range" not in bazel_rules: "@com_google_protobuf//third_party/utf8_range:utf8_range" ] _BUILD_EXTRA_METADATA["@utf8_range//:utf8_range"] = md -all_extra_metadata.update(_BUILD_EXTRA_METADATA) all_extra_metadata.update( _generate_build_extra_metadata_for_tests(tests, bazel_rules) ) +all_extra_metadata.update(_BUILD_EXTRA_METADATA) # Step 4: Compute the build metadata that will be used in the final build.yaml. # The final build metadata includes transitive dependencies, and sources/headers diff --git a/tools/buildgen/plugins/check_attrs.py b/tools/buildgen/plugins/check_attrs.py index 8f14277f433..6d051f36466 100644 --- a/tools/buildgen/plugins/check_attrs.py +++ b/tools/buildgen/plugins/check_attrs.py @@ -85,6 +85,7 @@ VALID_ATTRIBUTE_KEYS_MAP = { "language": one_of(("c", "c89", "c++", "csharp")), "maxlen": anything(), "platforms": subset_of(("linux", "mac", "posix", "windows")), + "plugin_option": anything(), "run": one_of((True, False)), "secure": one_of(("check", True, False)), "src": anything(), diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 8965c31bb25..49f6bb44395 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -6455,30 +6455,6 @@ ], "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": "otel_plugin_test", - "platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "uses_polling": true - }, { "args": [], "benchmark": false,