refactor: improve telemetry api receiver listener initialization logic#2096
Conversation
| if err != nil { | ||
| return nil, "", err | ||
| } | ||
| addr := fmt.Sprintf("%s:%d", l.Addr().Network(), l.Addr().(*net.TCPAddr).Port) |
There was a problem hiding this comment.
Looks like l.Addr().Network() returns the network type string "tcp". So wouldn't this produce an address like tcp:53421 instead of sandbox.localdomain:53421?
That would mean the subscribe URI passed to the Telemetry API ends up as http://tcp:53421/, which Lambda wouldn't be able to resolve to deliver events to I think?
For reference, the internal listener at internal/telemetryapi/listener.go uses the listenerAddr variable directly for this and I think a lot of the logic we already have there is directly applicable here as well.
There was a problem hiding this comment.
Good catch, this must have been a typo, or I made this change very late at night. I can update this, and add a test, since I'm surprised this managed to slip through.
|
@herin049 if you get a chance to check out the feedback again, we can merge this sometime soon. |
No rush! Just wanted to make sure it’s still on your radar. |
herin049
force-pushed
the
refactor/improve-telemetryreceiver-listener
branch
from
bde0377 to
46ecc39
Compare
herin049
force-pushed
the
refactor/improve-telemetryreceiver-listener
branch
from
1bc885f to
fce77c2
Compare
2 participants
Improves the Telemetry API receiver listener initialization logic to more closely match the logic used elsewhere in the layer. Updated the default behavior to listen on
:0by default to eliminate the need of manually specifying ports and eliminate the possibility of encounteringError: address already in useerrors that are very rare and difficult to debug. The listener now handles shutdown properly by closing the HTTP listener, and several incorrect log statements have been updated with a few additional ones added.