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

otlploggrpc: Remove unnecessary grpc client setup options #5273

Closed
wants to merge 2 commits into from

Conversation

XSAM
Copy link
Member

@XSAM XSAM commented Apr 27, 2024

part of #5056

Based on the recent discussion at the SIG meeting, we only provide the minimum option, WithGRPCConn, for the grpc client to remove the burden of catching up on the latest grpc client option.

@XSAM XSAM added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Apr 27, 2024
@XSAM XSAM changed the title Remove grpc client setup options otlploggrpc: Remove unnecessary grpc client setup options Apr 27, 2024
@XSAM XSAM mentioned this pull request Apr 27, 2024
18 tasks
// connection, just like grpc.WithInsecure()
// (https://pkg.go.dev/google.golang.org/grpc#WithInsecure) does.
//
// If the OTEL_EXPORTER_OTLP_ENDPOINT or OTEL_EXPORTER_OTLP_LOGS_ENDPOINT
Copy link
Member

Choose a reason for hiding this comment

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

We must remember to document all the env vars in Expoter type doc comments or package documentation.
This can be handled as a separate issue.

// grpc.DialOptions are ignored.
//
// This option has no effect if WithGRPCConn is used.
func WithDialOption(_ ...grpc.DialOption) Option {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should consider keeping DialOptions: #5248 (comment)

@XSAM
Copy link
Member Author

XSAM commented May 2, 2024

Closing this, as an existing use case mentioned in this #5248 (comment) won't be able to support if we merge this PR.

However, We should consider this PR in v2.

@XSAM XSAM closed this May 2, 2024
@XSAM XSAM deleted the remove-grpc-options branch May 2, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants