-
Notifications
You must be signed in to change notification settings - Fork 93
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
Jaeger Middleware: Missing Span Context in Proxied Request Header #110
Conversation
Hi @carlosedp can you please review? |
@jamesstocktonj1 could you provide example for for these 2 small apps and docker command to run jaeger. I have no experience with Jaeger and I would like to avoid merging things without even being able to see/test them out myself. |
Hi James, thanks for tagging me but I've been a little away from Echo and Go recently... |
Hi all, thanks for the reply. I have created a quick project to demonstrate the issue. |
Thank you @jamesstocktonj1 Your example project was helpful. I did not have Jaeger experience. For history sake for myself. These were steps to reproduce. I simplified that example project to
docker run --rm -d --network host --name my_jaeger jaegertracing/all-in-one:latest
package main
import (
"log"
"net/url"
"os"
"github.com/labstack/echo-contrib/jaegertracing"
"github.com/labstack/echo/v4"
"github.com/labstack/echo/v4/middleware"
)
// docker run --rm -d --network host --name my_jaeger jaegertracing/all-in-one:latest
func main() {
os.Setenv("JAEGER_AGENT_HOST", "localhost")
os.Setenv("JAEGER_AGENT_PORT", "6831")
os.Setenv("JAEGER_SERVICE_NAME", "echo-proxy-1")
// basic echo server
s := echo.New()
s.Use(middleware.Logger())
s.Use(middleware.Recover())
// init jaeger middleware
closer := jaegertracing.New(s, nil)
defer closer.Close()
// init proxy middleware
urlString := []string{
"http://localhost:8081",
}
urls, err := parseUrls(urlString)
if err != nil {
log.Fatal(err)
}
s.Use(middleware.Proxy(middleware.NewRoundRobinBalancer(urls)))
// start server
s.Logger.Fatal(s.Start(":8080"))
}
func parseUrls(urls []string) ([]*middleware.ProxyTarget, error) {
var targets []*middleware.ProxyTarget
for _, u := range urls {
u, err := url.Parse(u)
if err != nil {
return nil, err
}
targets = append(targets, &middleware.ProxyTarget{URL: u})
}
return targets, nil
}
package main
import (
"net/http"
"os"
"github.com/labstack/echo-contrib/jaegertracing"
"github.com/labstack/echo/v4"
"github.com/labstack/echo/v4/middleware"
)
// docker run --rm -d --network host --name my_jaeger jaegertracing/all-in-one:latest
func main() {
os.Setenv("JAEGER_AGENT_HOST", "localhost")
os.Setenv("JAEGER_AGENT_PORT", "6831")
os.Setenv("JAEGER_SERVICE_NAME", "echo-server")
// basic echo server
s := echo.New()
s.Use(middleware.Logger())
s.Use(middleware.Recover())
// init jaeger middleware
closer := jaegertracing.New(s, nil)
defer closer.Close()
s.GET("/hello", func(c echo.Context) error {
return c.String(http.StatusOK, "Hello, World!")
})
// start server
s.Logger.Fatal(s.Start(":8081"))
}
curl localhost:8080/hello
|
I have tagged new release. https://github.com/labstack/echo-contrib/releases/tag/v0.17.0 |
…226) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [github.com/labstack/echo-contrib](https://togithub.com/labstack/echo-contrib) | `v0.16.0` -> `v0.17.1` | [![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2flabstack%2fecho-contrib/v0.17.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2flabstack%2fecho-contrib/v0.17.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2flabstack%2fecho-contrib/v0.16.0/v0.17.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2flabstack%2fecho-contrib/v0.16.0/v0.17.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>labstack/echo-contrib (github.com/labstack/echo-contrib)</summary> ### [`v0.17.1`](https://togithub.com/labstack/echo-contrib/releases/tag/v0.17.1) [Compare Source](https://togithub.com/labstack/echo-contrib/compare/v0.17.0...v0.17.1) #### What's Changed **Security** - Update golang.org/x/net dependency because of [GO-2024-2687](https://pkg.go.dev/vuln/GO-2024-2687) by [@​aldas](https://togithub.com/aldas) in [https://github.com/labstack/echo-contrib/pull/115](https://togithub.com/labstack/echo-contrib/pull/115) **Enhancements** - Upgrade dependencies by [@​aldas](https://togithub.com/aldas) in [https://github.com/labstack/echo-contrib/pull/115](https://togithub.com/labstack/echo-contrib/pull/115) **Full Changelog**: labstack/echo-contrib@v0.17.0...v0.17.1 ### [`v0.17.0`](https://togithub.com/labstack/echo-contrib/releases/tag/v0.17.0) [Compare Source](https://togithub.com/labstack/echo-contrib/compare/v0.16.0...v0.17.0) #### What's Changed - Jaeger Middleware: Missing Span Context in Proxied Request Header by [@​jamesstocktonj1](https://togithub.com/jamesstocktonj1) in [https://github.com/labstack/echo-contrib/pull/110](https://togithub.com/labstack/echo-contrib/pull/110) #### New Contributors - [@​jamesstocktonj1](https://togithub.com/jamesstocktonj1) made their first contribution in [https://github.com/labstack/echo-contrib/pull/110](https://togithub.com/labstack/echo-contrib/pull/110) **Full Changelog**: labstack/echo-contrib@v0.16.0...v0.17.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View the [repository job log](https://developer.mend.io/github/infratographer/x). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yOTMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjQzOC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Description
I tried to create a simple Echo reverse proxy (echo/v4/middleware) which connects to 2 simple Echo services (Hello World), both with the Jaeger middleware connecting to the jaeger/all-in-one docker image. I noticed that each of the services was creating their own trace in Jaeger rather than the proxy propagating through to the hello world instance. The correct headers were not being set for the context to be propagated.
Before:
With the default settings, the
config.Tracer.Extract
function at the start of the Jaeger middleware is looking for the trace context in the header value "uber-trace-id" (jaeger.TraceContextHeaderName
). This fix adds the context value in the header by using theconfig.Tracer.Inject
function.After:
See also: serializing-to-the-wire
TLDR
Fix Jaeger middleware when used with a proxy by adding the trace context to the request header.