Skip to content

Client span generation for upstream requests #97

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

avahahn
Copy link

@avahahn avahahn commented May 21, 2025

Proposed changes

Fixes #17
See the below commit description for details:

This commit adds support to configure client span generation for
upstream requests. Client span generation can be configured per
upstream using a configuration snippet like the one below:

upstream echo.sunnypup.io {
    otel_upstream_span_enable;
    server echo.sunnypup.io:443;
}

The primary vehicle for this is the load balancer API. Existing
load balancer callbacks and data are stored in a new context type.
New callbacks are added for getting, freeing requests as well as
setting and saving ssl sessions. These modifications are made if
and only if the otel_upstream_span_enable configuration directive
is made (per upstream).

In addition to the above, A new enumeration is made in the batch
exporter for span type. Currently Client and Server are recognized,
however this can be extended as needed in the future. This enum is
matched with corresponding elements in the underlying opentelemetry
library.

The logic path between getOtelCtx/ensureOtelCtx/onRequestStart is
modified to make sure that even upstream calls made by subrequests
have the correct metadata needed for proper span propagation.

Additionally, some logic is refactored out of onRequestEnd for
reuse in the upstream span generation.

Finally, a new routine is added to set client span specific attrs.

Note: these changes do require that NGINX is compiled with SSL
support. This is needed because use of the load balancer API,
specifically wrapping around the data argument to callbacks, must
be handled properly when setting SSL sessions or else a memory
corruption error can be triggered by the round robin SSL handlers
recieving bad data.

Signed-off-by: Ava Hahn <[email protected]>

A second commit is included to provide additional testing:

This commit adds some loose testing for the functionality included
in the previous commit. A new test is minted that verifies the type,
quantity, and return code of client and server spans after a request
that leverages an upstream.

Signed-off-by: Ava Hahn <[email protected]>

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING document
  • If applicable, I have added tests that prove my fix is effective or that my feature works
  • If applicable, I have checked that any relevant tests pass after adding my changes

avahahn added 2 commits May 2, 2025 13:36
This commit adds support to configure client span generation for
upstream requests. Client span generation can be configured per
upstream using a configuration snippet like the one below:

upstream echo.sunnypup.io {
    otel_upstream_span_enable;
    server echo.sunnypup.io:443;
}

The primary vehicle for this is the load balancer API. Existing
load balancer callbacks and data are stored in a new context type.
New callbacks are added for getting, freeing requests as well as
setting and saving ssl sessions. These modifications are made if
and only if the otel_upstream_span_enable configuration directive
is made (per upstream).

In addition to the above, A new enumeration is made in the batch
exporter for span type. Currently Client and Server are recognized,
however this can be extended as needed in the future. This enum is
matched with corresponding elements in the underlying opentelemetry
library.

The logic path between getOtelCtx/ensureOtelCtx/onRequestStart is
modified to make sure that even upstream calls made by subrequests
have the correct metadata needed for proper span propagation.

Additionally, some logic is refactored out of onRequestEnd for
reuse in the upstream span generation.

Finally, a new routine is added to set client span specific attrs.

Note: these changes do require that NGINX is compiled with SSL
support. This is needed because use of the load balancer API,
specifically wrapping around the data argument to callbacks, must
be handled properly when setting SSL sessions or else a memory
corruption error can be triggered by the round robin SSL handlers
recieving bad data.

Signed-off-by: Ava Hahn <[email protected]>
This commit adds some loose testing for the functionality included
in the previous commit. A new test is minted that verifies the type,
quantity, and return code of client and server spans after a request
that leverages an upstream.

Signed-off-by: Ava Hahn <[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
Development

Successfully merging this pull request may close these issues.

Add client spans around calls to upstream
1 participant