Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkg/gofr/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ const (
defaultPublicStaticDir = "static"
shutDownTimeout = 30 * time.Second
gofrTraceExporter = "gofr"
otlpTraceExporter = "otlp"
jaegerTraceExporter = "jaeger"
gofrTracerURL = "https://tracer.gofr.dev"
checkPortTimeout = 2 * time.Second
gofrHost = "https://gofr.dev"
Expand Down
2 changes: 1 addition & 1 deletion pkg/gofr/gofr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -836,7 +836,7 @@ func Test_initTracer_invalidConfig(t *testing.T) {
config config.Config
expectedLogMessage string
}{
{"unsupported trace_exporter", mockConfig1, "unsupported TRACE_EXPORTER: abc"},
{"unsupported trace_exporter", mockConfig1, "unsupported TRACE_EXPORTER=abc: expected one of otlp, jaeger, zipkin or gofr"},
{"missing trace_exporter", mockConfig2, "missing TRACE_EXPORTER config, should be provided with TRACER_URL to enable tracing"},
{"miss tracer_url ", mockConfig3,
"missing TRACER_URL config, should be provided with TRACE_EXPORTER to enable tracing"},
Expand Down
151 changes: 136 additions & 15 deletions pkg/gofr/otel.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package gofr
import (
"context"
"fmt"
"net/url"
"strconv"
"strings"
"unicode"

"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc"
Expand Down Expand Up @@ -170,8 +172,10 @@ func (a *App) tracerInsecure() (insecure, set bool) {

b, err := strconv.ParseBool(v)
if err != nil {
a.Logger().Warnf("invalid TRACER_INSECURE %q: expected a boolean; defaulting to plaintext for a "+
"schemeless TRACER_URL", v)
// The raw value is not echoed. Tracer config reaches a log only through
// redactURL, redactExporterName or as a matched constant.
a.Logger().Warn("invalid TRACER_INSECURE: expected true or false; defaulting to plaintext for a " +
"schemeless TRACER_URL")

return true, false
}
Expand All @@ -188,13 +192,10 @@ func (a *App) getExporter(name, host, port, url string) (sdktrace.SpanExporter,
headers := a.getTracerHeaders()

switch strings.ToLower(name) {
case "otlp", "jaeger":
// Resolved inside the case, not above the switch: TRACER_INSECURE means
// nothing to zipkin or gofr, and evaluating it for them would warn about a
// malformed value under an exporter that never reads it.
insecure, insecureSet := a.tracerInsecure()

exporter, err = buildOtlpExporter(a.Logger(), name, url, host, port, headers, insecure, insecureSet)
case otlpTraceExporter:
exporter, err = a.buildOtlpFamilyExporter(otlpTraceExporter, url, host, port, headers)
case jaegerTraceExporter:
exporter, err = a.buildOtlpFamilyExporter(jaegerTraceExporter, url, host, port, headers)
case "zipkin":
a.Logger().Warn("TRACE_EXPORTER=zipkin is deprecated and will be removed in a future release. " +
"Zipkin supports OTLP natively (v2.24+) — to migrate, switch to TRACE_EXPORTER=otlp " +
Expand All @@ -204,12 +205,25 @@ func (a *App) getExporter(name, host, port, url string) (sdktrace.SpanExporter,
case gofrTraceExporter:
exporter = buildGoFrExporter(a.Logger(), url)
default:
a.container.Errorf("unsupported TRACE_EXPORTER: %s", name)
a.container.Errorf("unsupported TRACE_EXPORTER=%s: expected one of otlp, jaeger, zipkin or gofr",
redactExporterName(name))
}

return exporter, err
}

// buildOtlpFamilyExporter builds the OTLP exporter for otlp and jaeger. The
// exporter name is passed as the matched constant rather than the configured
// string, so only a known name is ever logged.
func (a *App) buildOtlpFamilyExporter(name, endpoint, host, port string, headers map[string]string) (sdktrace.SpanExporter, error) {
// Resolved here, not in getExporter above the switch: TRACER_INSECURE means
// nothing to zipkin or gofr, and evaluating it for them would warn about a
// malformed value under an exporter that never reads it.
insecure, insecureSet := a.tracerInsecure()

return buildOtlpExporter(a.Logger(), name, endpoint, host, port, headers, insecure, insecureSet)
}

// otlpTransport is the resolved transport-security decision for the OTLP trace
// exporter: which endpoint option to use, whether to apply WithInsecure, and
// whether the resulting connection carries traffic in the clear.
Expand Down Expand Up @@ -266,7 +280,7 @@ func resolveOtlpTransport(logger logging.Logger, url string, insecure, insecureS
if strings.HasPrefix(scheme, schemeHTTP) || strings.HasPrefix(scheme, schemeHTTPS) {
if insecureSet {
logger.Warnf("TRACER_INSECURE is ignored for TRACER_URL=%q: transport security is derived "+
"from the URL scheme", url)
"from the URL scheme", redactURL(url))
}

return otlpTransport{useEndpointURL: true, plaintext: strings.HasPrefix(scheme, schemeHTTP)}
Expand All @@ -287,10 +301,10 @@ func buildOtlpExporter(logger logging.Logger, name, url, host, port string, head

if transport.plaintext && len(headers) > 0 {
logger.Warnf("traces are exported to %s over plaintext with headers configured: "+
"headers (including auth credentials) will be sent in the clear", url)
"headers (including auth credentials) will be sent in the clear", redactURL(url))
}

logger.Infof("Exporting traces to %s at %s", strings.ToLower(name), url)
logger.Infof("Exporting traces to %s at %s", name, redactURL(url))

var opts []otlptracegrpc.Option

Expand All @@ -316,7 +330,7 @@ func buildZipkinExporter(logger logging.Logger, url, host, port string, headers
url = fmt.Sprintf("http://%s:%s/api/v2/spans", host, port)
}

logger.Infof("Exporting traces to zipkin at %s", url)
logger.Infof("Exporting traces to zipkin at %s", redactURL(url))

var opts []zipkin.Option
if len(headers) > 0 {
Expand All @@ -331,11 +345,118 @@ func buildGoFrExporter(logger logging.Logger, url string) sdktrace.SpanExporter
url = "https://tracer-api.gofr.dev/api/spans"
}

logger.Infof("Exporting traces to GoFr at %s", url)
logger.Infof("Exporting traces to GoFr at %s", redactURL(url))

return NewExporter(url, logging.NewLogger(logging.INFO))
}

// redactedPlaceholder stands in for credentials when a tracer endpoint is logged.
const redactedPlaceholder = "REDACTED"

// redactURL returns a tracer endpoint that is safe to write to a log.
//
// A scheme-bearing TRACER_URL is a real URL, so it can carry credentials in its
Comment thread
aryanmehrotra marked this conversation as resolved.
// userinfo (https://user:token@collector:4317) or its query
// (https://collector/api/v2/spans?api-key=...). Both are replaced rather than
// dropped, so the log still shows that something was configured there.
//
// Secrets placed in path segments (https://host/v1/TOKEN/spans) are not
// redacted: no tracer backend GoFr supports takes credentials there, and
// hiding the path would hide the part of the endpoint an operator debugs.
//
// Control characters are escaped on the unparsed path only: url.Parse rejects
// them, so such a value always lands there, and a raw newline would otherwise
// forge a second log line in the terminal's pretty output.
func redactURL(raw string) string {
Comment thread
aryanmehrotra marked this conversation as resolved.
u, err := url.Parse(raw)
if err != nil || u.Host == "" {
return escapeControlChars(redactUnparsedURL(raw))
}

if u.User != nil {
u.User = url.User(redactedPlaceholder)
}

if u.RawQuery != "" {
u.RawQuery = redactedPlaceholder
}

u.Fragment = ""

return u.String()
}

// escapeControlChars replaces every control character with its \x or \u
// escape, so a logged value always stays on one line.
func escapeControlChars(s string) string {
if strings.IndexFunc(s, unicode.IsControl) < 0 {
return s
}

var b strings.Builder

for _, r := range s {
switch {
case !unicode.IsControl(r):
b.WriteRune(r)
case r <= unicode.MaxLatin1:
fmt.Fprintf(&b, `\x%02x`, r)
default:
fmt.Fprintf(&b, `\u%04x`, r)
}
}

return b.String()
}

// maxEchoedExporterNameLen bounds how much of an unsupported TRACE_EXPORTER is
// echoed back; every supported name is at most 6 characters.
const maxEchoedExporterNameLen = 16

// redactExporterName returns an unsupported TRACE_EXPORTER value in a form safe
// to log. A typo of a real exporter name -- short, letters and '-' or '_' only
// -- is echoed so it can be spotted; anything else, which is more likely a
// value pasted into the wrong variable, is replaced.
func redactExporterName(name string) string {
if name == "" || len(name) > maxEchoedExporterNameLen {
return redactedPlaceholder
}

for _, r := range name {
if (r < 'a' || r > 'z') && (r < 'A' || r > 'Z') && r != '-' && r != '_' {
return redactedPlaceholder
}
}

return name
}

// redactUnparsedURL redacts an endpoint that url.Parse cannot split into a host
// and the rest: a schemeless host:port, host/path or user:pass@host, or input
// that does not parse at all. It works on the raw string and over-redacts
// rather than risk a leak.
//
// Userinfo is handled first, up to the last '@'. A '?' in a password, or an
// '@' in a query value, then falls inside the replaced prefix instead of
// splitting the credential. The query and fragment are handled after that.
func redactUnparsedURL(raw string) string {
redacted := raw

if i := strings.LastIndex(redacted, "@"); i >= 0 {
redacted = redactedPlaceholder + redacted[i:]
}

if i := strings.IndexByte(redacted, '#'); i >= 0 {
redacted = redacted[:i]
}

if i := strings.IndexByte(redacted, '?'); i >= 0 {
redacted = redacted[:i+1] + redactedPlaceholder
}

return redacted
}

type otelErrorHandler struct {
logger logging.Logger
}
Expand Down
117 changes: 117 additions & 0 deletions pkg/gofr/otel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -614,3 +614,120 @@ func BenchmarkSpanStart_NeverSampleSDK(b *testing.B) {
span.End()
}
}

func Test_redactExporterName(t *testing.T) {
tests := []struct {
name string
value string
expected string
}{
{name: "typo of a real exporter is echoed", value: "otpl", expected: "otpl"},
{name: "mixed case and separators are echoed", value: "Open_Telemetry-x", expected: "Open_Telemetry-x"},
{name: "empty is replaced", value: "", expected: "REDACTED"},
{name: "longer than 16 is replaced", value: "abcdefghijklmnopq", expected: "REDACTED"},
{name: "digits look like a pasted secret", value: "ab12cd34", expected: "REDACTED"},
{name: "URL pasted into the wrong variable", value: "https://x", expected: "REDACTED"},
{name: "control character is replaced", value: "otlp\n", expected: "REDACTED"},
{name: "non-ASCII letter is replaced", value: "ötlp", expected: "REDACTED"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
require.Equal(t, tt.expected, redactExporterName(tt.value))
})
}
}

func Test_redactURL(t *testing.T) {
tests := []struct {
name string
raw string
expected string
}{
{name: "schemeless host:port is unchanged", raw: "collector:4317", expected: "collector:4317"},
{name: "plain https URL is unchanged", raw: "https://collector:4317", expected: "https://collector:4317"},
{name: "path is kept", raw: "http://localhost:2005/api/v2/spans", expected: "http://localhost:2005/api/v2/spans"},
{name: "empty is unchanged", raw: "", expected: ""},
{name: "userinfo with password", raw: "https://user:s3cret@collector:4317", expected: "https://REDACTED@collector:4317"},
{name: "userinfo token only", raw: "https://t0ken@collector:4317", expected: "https://REDACTED@collector:4317"},
{name: "query credentials", raw: "https://zipkin/api/v2/spans?api-key=s3cret", expected: "https://zipkin/api/v2/spans?REDACTED"},
{name: "fragment is dropped", raw: "https://collector:4317#s3cret", expected: "https://collector:4317"},
{name: "schemeless userinfo", raw: "user:s3cret@collector:4317", expected: "REDACTED@collector:4317"},
{name: "unparsable with userinfo", raw: "https://user:s3cret@collector:43%17", expected: "REDACTED@collector:43%17"},
{name: "schemeless query credentials", raw: "localhost:9411/api/v2/spans?api-key=s3cret",
expected: "localhost:9411/api/v2/spans?REDACTED"},
{name: "schemeless userinfo and query", raw: "user:s3cret@collector:4317/p?k=s3cret", expected: "REDACTED@collector:4317/p?REDACTED"},
{name: "schemeless password containing '?'", raw: "user:s3?cret@collector:4317", expected: "REDACTED@collector:4317"},
{name: "schemeless '@' inside query value", raw: "collector:4317/p?k=s3cret@x", expected: "REDACTED@x"},
{name: "schemeless fragment is dropped", raw: "collector:4317#s3cret", expected: "collector:4317"},
{name: "newline cannot forge a log line", raw: "collector:4317\n{\"level\":\"INFO\"}",
expected: `collector:4317\x0a{"level":"INFO"}`},
{name: "carriage return and tab are escaped", raw: "host\r:4317\t", expected: `host\x0d:4317\x09`},
{name: "C1 control is escaped", raw: "host" + string(rune(0x85)) + ":4317", expected: `host\x85:4317`},
{name: "control char with userinfo is escaped after redaction", raw: "https://user:s3cret@collector\n:4317",
expected: `REDACTED@collector\x0a:4317`},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
require.Equal(t, tt.expected, redactURL(tt.raw))
})
}
}

// Test_initTracer_doesNotLogCredentials drives the real startup path for every
// exporter that logs its endpoint, and asserts the credentials in TRACER_URL and
// the raw TRACER_INSECURE value never reach the log.
func Test_initTracer_doesNotLogCredentials(t *testing.T) {
const secret = "s3cret-value"

tests := []struct {
name string
exporter string
url string
insecure string
expected string
}{
{name: "otlp userinfo", exporter: "otlp", url: "https://user:" + secret + "@localhost:4317",
expected: "Exporting traces to otlp at https://REDACTED@localhost:4317"},
{name: "jaeger ignored TRACER_INSECURE", exporter: "JAEGER", url: "http://user:" + secret + "@localhost:4317",
insecure: "true", expected: "Exporting traces to jaeger at http://REDACTED@localhost:4317"},
{name: "otlp invalid TRACER_INSECURE", exporter: "otlp", url: "localhost:4317", insecure: secret,
expected: "invalid TRACER_INSECURE"},
{name: "zipkin query key", exporter: "zipkin", url: "http://localhost:2005/api/v2/spans?api-key=" + secret,
expected: "Exporting traces to zipkin at http://localhost:2005/api/v2/spans?REDACTED"},
{name: "zipkin schemeless query key", exporter: "zipkin", url: "localhost:9411/api/v2/spans?api-key=" + secret,
expected: "Exporting traces to zipkin at localhost:9411/api/v2/spans?REDACTED"},
{name: "gofr userinfo", exporter: "gofr", url: "https://user:" + secret + "@tracer.example.com/api/spans",
expected: "Exporting traces to GoFr at https://REDACTED@tracer.example.com/api/spans"},
{name: "secret pasted into TRACE_EXPORTER", exporter: secret, url: "localhost:4317",
expected: "unsupported TRACE_EXPORTER=REDACTED"},
{name: "typo in TRACE_EXPORTER is echoed", exporter: "otpl", url: "localhost:4317",
expected: "unsupported TRACE_EXPORTER=otpl"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cfg := map[string]string{"TRACE_EXPORTER": tt.exporter, "TRACER_URL": tt.url, "TRACER_AUTH_KEY": "Bearer x"}
if tt.insecure != "" {
cfg["TRACER_INSECURE"] = tt.insecure
}

var stderr string

stdout := testutil.StdoutOutputForFunc(func() {
stderr = testutil.StderrOutputForFunc(func() {
mockContainer, _ := container.NewMockContainer(t)

a := App{Config: config.NewMockConfig(cfg), container: mockContainer}
a.initTracer()
})
})

out := stdout + stderr

require.Contains(t, out, tt.expected)
require.NotContains(t, out, secret)
})
}
}