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.

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

Problem with logging after introducing logr #4141

Closed
antoniomacri opened this issue May 26, 2023 · 0 comments · Fixed by #4143
Closed

Problem with logging after introducing logr #4141

antoniomacri opened this issue May 26, 2023 · 0 comments · Fixed by #4143
Assignees
Labels
bug Something isn't working
Projects

Comments

@antoniomacri
Copy link

Description

It seems that the logger is being called with its varargs as format arguments, but logr wants a list of key-values.

Environment

module main

go 1.19

require (
	github.com/go-logr/stdr v1.2.2
	go.opentelemetry.io/otel v1.16.0
	go.opentelemetry.io/otel/exporters/zipkin v1.16.0
	go.opentelemetry.io/otel/sdk v1.16.0
	go.opentelemetry.io/otel/trace v1.16.0
)

require (
	github.com/go-logr/logr v1.2.4 // indirect
	github.com/openzipkin/zipkin-go v0.4.1 // indirect
	go.opentelemetry.io/otel/metric v1.16.0 // indirect
	golang.org/x/sys v0.8.0 // indirect
)

Steps To Reproduce

package main

import (
	"context"
	"github.com/go-logr/stdr"
	"go.opentelemetry.io/otel/codes"
	"go.opentelemetry.io/otel/exporters/zipkin"
	sdktrace "go.opentelemetry.io/otel/sdk/trace"
	"go.opentelemetry.io/otel/sdk/trace/tracetest"
	"go.opentelemetry.io/otel/trace"
	"log"
	"os"
	"time"
)

func main() {
	exporter, err := zipkin.New("http://localhost:9411/api/v2/spans", zipkin.WithLogr(stdr.New(log.New(os.Stdout, "", 0))))
	if err != nil {
		log.Fatalf("cannot initialize the application tracer: %v1", err)
	}
	err = exporter.ExportSpans(context.Background(), tracetest.SpanStubs{{
		SpanContext: trace.NewSpanContext(trace.SpanContextConfig{
			TraceID: trace.TraceID{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F},
			SpanID:  trace.SpanID{0xFF, 0xFE, 0xFD, 0xFC, 0xFB, 0xFA, 0xF9, 0xF8},
		}),
		SpanKind:   trace.SpanKindServer,
		Name:       "foo",
		StartTime:  time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC),
		EndTime:    time.Date(2020, time.March, 11, 19, 25, 0, 0, time.UTC),
		Attributes: nil,
		Events:     nil,
		Status: sdktrace.Status{
			Code:        codes.Error,
			Description: "404, file not found",
		},
	}}.Snapshots())
	if err != nil {
		log.Fatalf("error: %v", err)
	}
}

It outputs this:

"level"=0 "msg"="about to send a POST request to %s with body %s" "<non-string-key: [\"http://localho>"="<no-value>"

Expected behavior

A structured log, something like:

"level"=0 "msg"="about to send a POST request" "url"="http://localhost:9411/api/v2/spans" "body"="..."
@antoniomacri antoniomacri added the bug Something isn't working label May 26, 2023
MrAlias added a commit to MrAlias/opentelemetry-go that referenced this issue May 27, 2023
Fixes open-telemetry#4141

The logr calling convention does not support fmt semantics for message
formatting. Format the message before it is sent to logr for logging.
@MrAlias MrAlias self-assigned this May 27, 2023
@MrAlias MrAlias added this to Needs triage in Bugs via automation May 27, 2023
@MrAlias MrAlias moved this from Needs triage to Low priority in Bugs May 27, 2023
MrAlias added a commit that referenced this issue Jun 26, 2023
* Format log message before logging with logr

Fixes #4141

The logr calling convention does not support fmt semantics for message
formatting. Format the message before it is sent to logr for logging.

* Add change to changelog

* Fix lint

Don't shadow the log pkg.

* Replace buflogr with funcr

* Run make

* Update exporters/zipkin/zipkin_test.go

Co-authored-by: Robert Pająk <pellared@hotmail.com>

---------

Co-authored-by: Robert Pająk <pellared@hotmail.com>
Bugs automation moved this from Low priority to Closed Jun 26, 2023
@MrAlias MrAlias added this to the v1.17.0/v0.40.0 milestone Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Bugs
  
Closed
Development

Successfully merging a pull request may close this issue.

2 participants