Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TEST] Functional tests for OTLP/gRPC with mutual TLS #3227

Merged
merged 6 commits into from
Jan 6, 2025

Conversation

chusitoo
Copy link
Contributor

@chusitoo chusitoo commented Jan 4, 2025

Fixes #2208

Changes

  • There was only a main for http functional tests so a func_grpc_main was added following a similar pattern
  • Add debug log line in OTLP grpc exporter to capture result in tests
  • Reuse existing CI step "cmake_otprotocol_test" since it is the only one enabling WITH_OTLP_GRPC_SSL_MTLS_PREVIEW
  • Minor tweaks to scripts to accommodate using both test binaries

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

Copy link

netlify bot commented Jan 4, 2025

Deploy Preview for opentelemetry-cpp-api-docs canceled.

Name Link
🔨 Latest commit 9a21128
🔍 Latest deploy log https://app.netlify.com/sites/opentelemetry-cpp-api-docs/deploys/677ae93723227700085106c9

Copy link

codecov bot commented Jan 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.16%. Comparing base (4998eb1) to head (9a21128).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3227   +/-   ##
=======================================
  Coverage   88.16%   88.16%           
=======================================
  Files         198      198           
  Lines        6224     6224           
=======================================
  Hits         5487     5487           
  Misses        737      737           

@chusitoo chusitoo force-pushed the FunctionalTestMutualTLS branch from f2bce20 to f40c656 Compare January 4, 2025 23:15
@chusitoo chusitoo force-pushed the FunctionalTestMutualTLS branch from f40c656 to 542a112 Compare January 4, 2025 23:38
@chusitoo chusitoo marked this pull request as ready for review January 5, 2025 03:16
@chusitoo chusitoo requested a review from a team as a code owner January 5, 2025 03:16
export TEST_BIN_DIR=${BUILD_DIR}/functional/otlp/
export TEST_BIN_DIR="${BUILD_DIR}/functional/otlp/"

[ ! -f "${TEST_BIN_DIR}/${TEST_EXECUTABLE}" ] && echo "::notice::Executable ${TEST_EXECUTABLE} not built in this configuration" && exit 0
Copy link
Contributor Author

@chusitoo chusitoo Jan 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this script gets its inputs from run_test.sh and all other CI checks that run these tests only build WITH_OTLP_HTTP, I thought this would be the easiest solution to supporting existing checks that only test HTTP functionality as well this check that does gRPC, as well.

For visibility, this message is also shown as a notice in the annotations section:

image

void set_common_opts(otlp::OtlpGrpcExporterOptions &opts)
{
opts.endpoint = opt_endpoint;
opts.timeout = std::chrono::milliseconds{100};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting a timeout to speed things up a bit, since some tests might go on for 5+ seconds before erroring out.
The observable behavior is that a "deadline exceeded" will be output as a result instead of the actual reason. This is accounted for in expect_connection_failed().

If deemed more appropriate, it could be reverted so as to let the tests fail normally.

@@ -452,6 +452,15 @@ jobs:
run: |
sudo ./ci/setup_grpc.sh
./ci/do_ci.sh cmake.exporter.otprotocol.test
- name: generate test cert
Copy link
Contributor Author

@chusitoo chusitoo Jan 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reusing this test because it is the only one enabling WITH_OTLP_GRPC_SSL_MTLS_PREVIEW and also WITH_OTLP_GRPC so it sort of made sense to add functional tests to it

Copy link
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the tests.

@marcalff marcalff merged commit bb68f49 into open-telemetry:main Jan 6, 2025
57 checks passed
malkia added a commit to malkia/opentelemetry-cpp that referenced this pull request Jan 6, 2025
[TEST] Functional tests for OTLP/gRPC with mutual TLS  (open-telemetry#3227)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TEST] Functional tests for OTLP GRPC MTLS export
2 participants