Skip to content

otelconf: add tests for grpc exporters with different certificate configurations #7233

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

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

bacherfl
Copy link
Contributor

This PR adds component tests for the otlp grpc exporters for logs/metrics/traces. The tests work by starting a simple grpc server that implements the Export interface of the respective signals, and records what is being sent to it. This should server as a way to verify that the otlp exporters created within this package are able to communicate with the server.

Closes #7222

@github-actions github-actions bot requested review from codeboten and pellared April 16, 2025 09:56
Copy link

codecov bot commented Apr 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.1%. Comparing base (23d3857) to head (8d398c8).
Report is 19 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #7233     +/-   ##
=======================================
+ Coverage   81.0%   81.1%   +0.1%     
=======================================
  Files        204     204             
  Lines      18080   18148     +68     
=======================================
+ Hits       14651   14729     +78     
+ Misses      3003    3000      -3     
+ Partials     426     419      -7     

see 10 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bacherfl bacherfl marked this pull request as ready for review April 22, 2025 09:17
@bacherfl bacherfl requested a review from a team as a code owner April 22, 2025 09:17
@bacherfl
Copy link
Contributor Author

@pellared this should be ready for review now - I'm not sure if a changelog entry is needed as this PR only adds tests.
The failing lint check seems to affect files that are not part of this PR, here I'm not sure how to address that

@pellared pellared added the Skip Changelog Allow PR to succeed without requiring an addition to the CHANGELOG label Apr 22, 2025
@pellared
Copy link
Member

The failing lint check seems to affect files that are not part of this PR, here I'm not sure how to address that

From build failure

Working tree is not clean, did you forget to run "make precommit"?

Guessing that go mod tidy run is missing as modified: otelconf/go.mod 😉

Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

Comment on lines 800 to 803
n, err := net.Listen("tcp", "localhost:0")
require.NoError(t, err)

tt.args.otlpConfig.Endpoint = ptr(strings.ReplaceAll(n.Addr().String(), "127.0.0.1", "localhost"))
Copy link
Member

@pellared pellared Apr 28, 2025

Choose a reason for hiding this comment

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

Isn't this a workaround?
Shouldn't this work?

n, err := net.Listen("tcp", "127.0.0.1:0")
require.NoError(t, err)
tt.args.otlpConfig.Endpoint = ptr(n.Addr().String())
    --- FAIL: Test_otlpGRPCLogExporter/with_TLS_config_and_client_key (0.00s)
        /home/hello-there/repos/opentelemetry-go-contrib/otelconf/v0.3.0/log_test.go:814: 
            	Error Trace:	/home/hello-there/repos/opentelemetry-go-contrib/otelconf/v0.3.0/log_test.go:814
            	Error:      	Received unexpected error:
            	            	parse "127.0.0.1:44735": invalid URI for request
            	Test:       	Test_otlpGRPCLogExporter/with_TLS_config_and_client_key

Shouldn't we create a bug that the confiugration is not working e.g. for 127.0.0.1:4318?

From https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#configuration-options:

The option SHOULD accept any form allowed by the underlying gRPC client implementation.

CC @codeboten

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I added a comment for now indicating that this is a workaround - forgot to include that when I first stumbled over this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog Allow PR to succeed without requiring an addition to the CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

otelconf: Increase test coverage for OTLP-related code
3 participants