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

cmd/telemetrygen: Add headers to gRPC metadata for logs #30082

Conversation

metalmatze
Copy link
Contributor

Description:

So far it wasn't possible to send (http) headers along with gRPC requests.
For that reason sending bearer tokens for authentication didn't work either.

This is fixed now.

@metalmatze metalmatze requested a review from a team December 19, 2023 11:02
@github-actions github-actions bot added the cmd/telemetrygen telemetrygen command label Dec 19, 2023
Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

This needs a changelog note

@@ -28,24 +29,32 @@ func newExporter(ctx context.Context, cfg *Config) (exporter, error) {
}, nil
}

if !cfg.Insecure {
return nil, fmt.Errorf("'telemetrygen logs' only supports insecure gRPC")
// TODO: Use an otlplogs grpc package in the future
Copy link
Member

Choose a reason for hiding this comment

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

Can you file an issue for this, explaining why we can't do this yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't able to find such a package. Does it exist?

Copy link
Member

Choose a reason for hiding this comment

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

It does not, I just want to have an issue in this repository tracking this work, since TODOs are often missed :) You can just say "We should switch to opentelemetry-go logs exporter once available, see open-telemetry/opentelemetry-go#4696 for the current state"

cmd/telemetrygen/internal/logs/exporter.go Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Jan 5, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jan 5, 2024
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Jan 19, 2024
@metalmatze
Copy link
Contributor Author

I've pushed an update to this PR's branch. Can we open this PR again?

@mx-psi
Copy link
Member

mx-psi commented Apr 1, 2024

@metalmatze Doesn't seem like I can :(

image

The aria label says "The telemetrygen-logs-grpc-metadata branch was force-pushed or recreated."

Would you mind opening a new PR and pinging me there?

mx-psi pushed a commit that referenced this pull request Apr 25, 2024
**Description:** <Describe what has changed.>
So far, sending (HTTP) headers along with gRPC requests hasn't been
possible.
For that reason, sending bearer tokens for authentication didn't work
either.

This is fixed now.

Updated version of #30082
cc @mx-psi

---------

Co-authored-by: Yang Song <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmd/telemetrygen telemetrygen command Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants