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

Added protocol handling for ServicePort and fixed type/value naming inside generic receiver #2619

Merged
merged 4 commits into from
Feb 14, 2024

Conversation

oldium
Copy link
Contributor

@oldium oldium commented Feb 12, 2024

Description:

Fixes three bugs:

  1. The "type/value" naming format was wrongly handled inside common receiver code (used by generic receiver)
  2. The Protocol (TCP/UDP) was not detected when creating K8s ServicePort
  3. The same port number could not be exposed with different protocol

Link to tracking Issue:

resolves: #2597
resolves: #767

Testing:

Added tests and also tested udplog and tcplog in our deployment.

Documentation:

This is purely a bugfix, so no documentation.

@oldium oldium requested a review from a team February 12, 2024 19:38
@oldium
Copy link
Contributor Author

oldium commented Feb 13, 2024

Changelog pipeline failed, I will add an entry mentioning fix of the #2597

@oldium
Copy link
Contributor Author

oldium commented Feb 13, 2024

Working on lint...

@oldium
Copy link
Contributor Author

oldium commented Feb 13, 2024

Changelog done, Lint done

The name is not correctly split before it is checked. Split it first.

Signed-off-by: Oldřich Jedlička <[email protected]>
The generic implementation does not allow to specify different endpoints
and endpoint-specific protocols, so split the implementation instead of
trying to make the generic implementation a complex catch-all-party.

Signed-off-by: Oldřich Jedlička <[email protected]>
It is legal to have the same protocol number and different protocol, so
handle it accordingly. Also interpret empty protocol as TCP, like the K8s
does.

Signed-off-by: Oldřich Jedlička <[email protected]>
Signed-off-by: Oldřich Jedlička <[email protected]>
@oldium
Copy link
Contributor Author

oldium commented Feb 14, 2024

Done. Anything more?

@oldium
Copy link
Contributor Author

oldium commented Feb 14, 2024

I see that End-to-end tests e2e on K8s 1.29 test failed, but I cannot find anything specific to this Pull Request.

@pavolloffay
Copy link
Member

maybe it was just flaky

@jaronoff97 jaronoff97 merged commit 822a24f into open-telemetry:main Feb 14, 2024
29 checks passed
@jaronoff97
Copy link
Contributor

thank you for your contribution!

ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
…nside generic receiver (open-telemetry#2619)

* Fix handling of type

The name is not correctly split before it is checked. Split it first.

Signed-off-by: Oldřich Jedlička <[email protected]>

* Split parsing of Syslog, TCP Log and UDP Log parsers

The generic implementation does not allow to specify different endpoints
and endpoint-specific protocols, so split the implementation instead of
trying to make the generic implementation a complex catch-all-party.

Signed-off-by: Oldřich Jedlička <[email protected]>

* Handle port numbers together with protocols

It is legal to have the same protocol number and different protocol, so
handle it accordingly. Also interpret empty protocol as TCP, like the K8s
does.

Signed-off-by: Oldřich Jedlička <[email protected]>

* Changelog updates

Signed-off-by: Oldřich Jedlička <[email protected]>

---------

Signed-off-by: Oldřich Jedlička <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants