Skip to content

Commit

Permalink
Merge pull request #6884 from timmysilv/tracing-endpoint
Browse files Browse the repository at this point in the history
jaeger-endpoint feature for non-agent trace collectors
  • Loading branch information
k8s-ci-robot authored Mar 9, 2021
2 parents b0b14d0 + a6442fb commit c90d33c
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 2 deletions.
5 changes: 5 additions & 0 deletions docs/user-guide/nginx-configuration/configmap.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ The following table shows a configuration option's name, type, and the default v
|[zipkin-sample-rate](#zipkin-sample-rate)|float|1.0|
|[jaeger-collector-host](#jaeger-collector-host)|string|""|
|[jaeger-collector-port](#jaeger-collector-port)|int|6831|
|[jaeger-endpoint](#jaeger-endpoint)|string|""|
|[jaeger-service-name](#jaeger-service-name)|string|"nginx"|
|[jaeger-sampler-type](#jaeger-sampler-type)|string|"const"|
|[jaeger-sampler-param](#jaeger-sampler-param)|string|"1"|
Expand Down Expand Up @@ -845,6 +846,10 @@ Specifies the host to use when uploading traces. It must be a valid URL.

Specifies the port to use when uploading traces. _**default:**_ 6831

## jaeger-endpoint

Specifies the endpoint to use when uploading traces to a collector. This takes priority over `jaeger-collector-host` if both are specified.

## jaeger-service-name

Specifies the service name to use for any traces created. _**default:**_ nginx
Expand Down
4 changes: 4 additions & 0 deletions docs/user-guide/third-party-addons/opentracing.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ jaeger-collector-host: jaeger-agent.default.svc.cluster.local
datadog-collector-host: datadog-agent.default.svc.cluster.local
```
NOTE: While the option is called `jaeger-collector-host`, you will need to point this to a `jaeger-agent`, and not the `jaeger-collector` component.
Alternatively, you can set `jaeger-endpoint` and specify the full endpoint for uploading traces. This will use TCP and should be used for a collector rather than an agent.

Next you will need to deploy a distributed tracing system which uses OpenTracing.
[Zipkin](https://github.com/openzipkin/zipkin) and
Expand Down Expand Up @@ -57,6 +58,9 @@ zipkin-sample-rate
# specifies the port to use when uploading traces, Default: 6831
jaeger-collector-port
# specifies the endpoint to use when uploading traces to a collector instead of an agent
jaeger-endpoint
# specifies the service name to use for any traces created, Default: nginx
jaeger-service-name
Expand Down
3 changes: 3 additions & 0 deletions internal/ingress/controller/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,9 @@ type Configuration struct {
// Default: 6831
JaegerCollectorPort int `json:"jaeger-collector-port"`

// JaegerEndpoint specifies the enpoint to use when uploading traces to a collector over TCP
JaegerEndpoint string `json:"jaeger-endpoint"`

// JaegerServiceName specifies the service name to use for any traces created
// Default: nginx
JaegerServiceName string `json:"jaeger-service-name"`
Expand Down
3 changes: 2 additions & 1 deletion internal/ingress/controller/nginx.go
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,7 @@ const jaegerTmpl = `{
"samplingServerURL": "{{ .JaegerSamplerHost }}:{{ .JaegerSamplerPort }}/sampling"
},
"reporter": {
"endpoint": "{{ .JaegerEndpoint }}",
"localAgentHostPort": "{{ .JaegerCollectorHost }}:{{ .JaegerCollectorPort }}"
},
"headers": {
Expand Down Expand Up @@ -1068,7 +1069,7 @@ func createOpentracingCfg(cfg ngx_config.Configuration) error {
if err != nil {
return err
}
} else if cfg.JaegerCollectorHost != "" {
} else if cfg.JaegerCollectorHost != "" || cfg.JaegerEndpoint != "" {
tmpl, err = template.New("jaeger").Parse(jaegerTmpl)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion internal/ingress/controller/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -1018,7 +1018,7 @@ func buildOpentracing(c interface{}, s interface{}) string {
buf.WriteString("opentracing_load_tracer /usr/local/lib64/libdd_opentracing.so /etc/nginx/opentracing.json;")
} else if cfg.ZipkinCollectorHost != "" {
buf.WriteString("opentracing_load_tracer /usr/local/lib/libzipkin_opentracing_plugin.so /etc/nginx/opentracing.json;")
} else if cfg.JaegerCollectorHost != "" {
} else if cfg.JaegerCollectorHost != "" || cfg.JaegerEndpoint != "" {
buf.WriteString("opentracing_load_tracer /usr/local/lib/libjaegertracing_plugin.so /etc/nginx/opentracing.json;")
}

Expand Down
21 changes: 21 additions & 0 deletions internal/ingress/controller/template/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1163,6 +1163,16 @@ func TestBuildOpenTracing(t *testing.T) {
t.Errorf("Expected '%v' but returned '%v'", expected, actual)
}

cfgNoHost := config.Configuration{
EnableOpentracing: true,
}
expected = "\r\n"
actual = buildOpentracing(cfgNoHost, []*ingress.Server{})

if expected != actual {
t.Errorf("Expected '%v' but returned '%v'", expected, actual)
}

cfgJaeger := config.Configuration{
EnableOpentracing: true,
JaegerCollectorHost: "jaeger-host.com",
Expand Down Expand Up @@ -1196,6 +1206,17 @@ func TestBuildOpenTracing(t *testing.T) {
t.Errorf("Expected '%v' but returned '%v'", expected, actual)
}

cfgJaegerEndpoint := config.Configuration{
EnableOpentracing: true,
JaegerEndpoint: "http://jaeger-collector.com:14268/api/traces",
}
expected = "opentracing_load_tracer /usr/local/lib/libjaegertracing_plugin.so /etc/nginx/opentracing.json;\r\n"
actual = buildOpentracing(cfgJaegerEndpoint, []*ingress.Server{})

if expected != actual {
t.Errorf("Expected '%v' but returned '%v'", expected, actual)
}

cfgOpenTracing := config.Configuration{
EnableOpentracing: true,
DatadogCollectorHost: "datadog-host.com",
Expand Down
15 changes: 15 additions & 0 deletions test/e2e/settings/opentracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const (

jaegerCollectorHost = "jaeger-collector-host"
jaegerSamplerHost = "jaeger-sampler-host"
// jaegerEndpoint = "jaeger-endpoint"

datadogCollectorHost = "datadog-collector-host"

Expand Down Expand Up @@ -174,6 +175,20 @@ var _ = framework.IngressNginxDescribe("Configure OpenTracing", func() {
assert.NotContains(ginkgo.GinkgoT(), log, "Unexpected failure reloading the backend", "reloading nginx after a configmap change")
})

/*
ginkgo.It("should enable opentracing using jaeger with an HTTP endpoint", func() {
config := map[string]string{}
config[enableOpentracing] = "true"
config[jaegerEndpoint] = "http://127.0.0.1/api/traces"
f.SetNginxConfigMapData(config)
framework.Sleep(10 * time.Second)
log, err := f.NginxLogs()
assert.Nil(ginkgo.GinkgoT(), err, "obtaining nginx logs")
assert.NotContains(ginkgo.GinkgoT(), log, "Unexpected failure reloading the backend", "reloading nginx after a configmap change")
})
*/

ginkgo.It("should enable opentracing using datadog", func() {
config := map[string]string{}
config[enableOpentracing] = "true"
Expand Down

0 comments on commit c90d33c

Please sign in to comment.