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

feat(mockotlpserver): add http tunnel option #608

Merged
merged 15 commits into from
Feb 27, 2025
Merged

Conversation

david-luna
Copy link
Member

No description provided.

@trentm
Copy link
Member

trentm commented Feb 14, 2025

Nice. Some notes:

@trentm
Copy link
Member

trentm commented Feb 14, 2025

Also not sure --proxy is the correct term here. Per https://datatracker.ietf.org/doc/html/rfc7230#section-2.3 this feature is more like a tunnel.

(Using --proxy as the optional name could be confusing. Compare, for example, to curl --proxy ... which is about tell curl to use that proxy to send all its outgoing requests to.)

@david-luna david-luna changed the title feat(mockotlpserver): add http proxy option feat(mockotlpserver): add http tunnel option Feb 21, 2025
@david-luna david-luna requested a review from trentm February 24, 2025 14:52
@david-luna david-luna marked this pull request as ready for review February 24, 2025 14:53
@david-luna
Copy link
Member Author

@trentm I've changed the option to tunnel as you suggested. I though of adding the option of multiple destinations but for our testing use case one is enough. We could do some follow up PRs to:

  • add multiple destinations
  • support GRPC

Copy link
Member

@trentm trentm left a comment

Choose a reason for hiding this comment

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

I haven't tried it yet, but this looks great. One nit/suggestion.

Co-authored-by: Trent Mick <trent.mick@elastic.co>
@david-luna david-luna merged commit 8c10660 into main Feb 27, 2025
2 checks passed
@david-luna david-luna deleted the mock-otlp-server-proxy branch February 27, 2025 08:20
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.

2 participants