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.

Sign up for GitHub

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

Jump to bottom

Use PeerTransmission when forwarding spans to peers #1324

Closed
VinozzZ opened this issue Sep 12, 2024 · 0 comments · Fixed by #1332
Closed

Use PeerTransmission when forwarding spans to peers #1324

VinozzZ opened this issue Sep 12, 2024 · 0 comments · Fixed by #1332
Assignees
@MikeGoldsmith
Labels
type: bug Something isn't working
Milestone
v2.9

Comments

@VinozzZ
Copy link
Contributor

VinozzZ commented Sep 12, 2024 •
edited by kentquirk
Loading

Since Refinery records metrics separately for sending spans to upstream honeycomb and to its peers, we should ensure that the peerTransmission is used when forwarding spans to peers in collector.

trace redistribution on peer membership change:

refinery/collect/collect.go

Line 434 in 8d916ed

i.Transmission.EnqueueSpan(sp)

trace redistribution during shutdown:

refinery/collect/collect.go

Line 1034 in 8d916ed

i.Transmission.EnqueueSpan(sp)

@VinozzZ VinozzZ added the type: bug Something isn't working label Sep 12, 2024
@VinozzZ VinozzZ added this to the v2.9 milestone Sep 12, 2024
@VinozzZ VinozzZ changed the title Use PeerTransmission to forward spans to peers during shutdown Use PeerTransmission when forwarding spans to peers during shutdown Sep 12, 2024
@VinozzZ VinozzZ changed the title Use PeerTransmission when forwarding spans to peers during shutdown Use PeerTransmission when forwarding spans to peers Sep 12, 2024
@MikeGoldsmith MikeGoldsmith self-assigned this Sep 16, 2024
@MikeGoldsmith MikeGoldsmith mentioned this issue Sep 16, 2024
Merged
TylerHelmuth pushed a commit that referenced this issue Oct 16, 2024
@MikeGoldsmith @TylerHelmuth
fix: Use peer transmission during redistribute and shutdown events (#…
f8388c8 
…1332)

## Which problem is this PR solving?

Updates the collector use the configured peer transmission to send spans
to peers during redistribute and shutdown events. This ensures metrics
for sending to Honeycomb and peers are accurate.

The peer transmission is already configured and used by the router when
receiving spans that should be sent to a peer. This is used the same
transmission object after a span was originally accepted but then should
be forwarded to a peer.

- Closes #1324

## Short description of the changes
- Inject the already configured peer transmission during collector init
- Update redistribute and shutdown logic to use the peer transmission
- Update tests throughout to verify the peer transmission is used
correctly and also verify it can be injected correctly
- note: there were existing tests to verify redistribute and shutdown
behaviour and have been updated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@MikeGoldsmith MikeGoldsmith

Labels
type: bug Something isn't working
Projects
None yet
Milestone
v2.9
Development

Successfully merging a pull request may close this issue.

fix: Use peer transmission during redistribute and shutdown events honeycombio/refinery
2 participants
@MikeGoldsmith @VinozzZ