Repository: quic-go/quic-go
Pull request #4556 (2024-06-05 07:36:48)

qlog: rename DefaultTracer to DefaultConnectionTracer
There's a `logging.Tracer` and a `logging.ConnectionTracer`. It's confusing that `qlog.DefaultTracer` returns a `ConnectionTracer`.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/quic-go/quic-go/pull/4556?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) Report
Attention: Patch coverage is `33.33333%` with `2 lines` in your changes missing coverage. Please review.
> Project coverage is 85.15%. Comparing base [(`44e0147`)](https://app.codecov.io/gh/quic-go/quic-go/commit/44e0147f2eb8b46bf55309afdd90cb2479614e8e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) to head [(`746c9f7`)](https://app.codecov.io/gh/quic-go/quic-go/commit/746c9f76b1fb211940f9acaf036ffc19eef33287?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).

| [Files](https://app.codecov.io/gh/quic-go/quic-go/pull/4556?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) | Patch % | Lines |
|---|---|---|
| [qlog/qlog\_dir.go](https://app.codecov.io/gh/quic-go/quic-go/pull/4556?src=pr&el=tree&filepath=qlog%2Fqlog_dir.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go#diff-cWxvZy9xbG9nX2Rpci5nbw==) | 33.33% | [2 Missing :warning: ](https://app.codecov.io/gh/quic-go/quic-go/pull/4556?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #4556      +/-   ##
==========================================
+ Coverage   85.12%   85.15%   +0.03%     
==========================================
  Files         154      154              
  Lines       14841    14843       +2     
==========================================
+ Hits        12633    12639       +6     
+ Misses       1700     1697       -3     
+ Partials      508      507       -1     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/quic-go/quic-go/pull/4556?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).


Patch:
diff --git a/example/client/main.go b/example/client/main.go
index c759855a..409a3444 100644
--- a/example/client/main.go
+++ b/example/client/main.go
@@ -47,7 +47,7 @@ func main() {
 			KeyLogWriter:       keyLog,
 		},
 		QUICConfig: &quic.Config{
-			Tracer: qlog.DefaultTracer,
+			Tracer: qlog.DefaultConnectionTracer,
 		},
 	}
 	defer roundTripper.Close()
diff --git a/example/main.go b/example/main.go
index d242ba4e..bf87f810 100644
--- a/example/main.go
+++ b/example/main.go
@@ -169,7 +169,7 @@ func main() {
 					Handler: handler,
 					Addr:    bCap,
 					QUICConfig: &quic.Config{
-						Tracer: qlog.DefaultTracer,
+						Tracer: qlog.DefaultConnectionTracer,
 					},
 				}
 				err = server.ListenAndServeTLS(certFile, keyFile)
diff --git a/metrics/connection_tracer.go b/metrics/connection_tracer.go
index adaee497..aa497b86 100644
--- a/metrics/connection_tracer.go
+++ b/metrics/connection_tracer.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"net"
+	"sync"
 	"time"
 
 	"github.com/quic-go/quic-go"
@@ -51,25 +52,27 @@ var (
 	)
 )
 
-// DefaultTracer returns a callback that creates a metrics ConnectionTracer.
-// The ConnectionTracer returned can be set on the quic.Config for a new connection.
-// It should be reused across QUIC connections.
-func DefaultTracer() func(_ context.Context, p logging.Perspective, _ logging.ConnectionID) *logging.ConnectionTracer {
-	return DefaultTracerWithRegisterer(prometheus.DefaultRegisterer)
-}
+var (
+	defaultServerConnectionTracerOnce, defaultClientConnectionTracerOnce sync.Once
+	defaultServerConnectionTracer, defaultClientConnectionTracer         *logging.ConnectionTracer
+)
 
-// DefaultTracerWithRegisterer returns a callback that creates a metrics ConnectionTracer
-// using a given Prometheus registerer.
-func DefaultTracerWithRegisterer(registerer prometheus.Registerer) func(_ context.Context, p logging.Perspective, _ logging.ConnectionID) *logging.ConnectionTracer {
-	return func(_ context.Context, p logging.Perspective, _ logging.ConnectionID) *logging.ConnectionTracer {
-		switch p {
-		case logging.PerspectiveClient:
-			return NewClientConnectionTracerWithRegisterer(registerer)
-		case logging.PerspectiveServer:
-			return NewServerConnectionTracerWithRegisterer(registerer)
-		default:
-			panic("invalid perspective")
-		}
+// DefaultConnectionTracer returns a callback that creates a metrics ConnectionTracer.
+// The ConnectionTracer returned can be set on the quic.Config for a new connection.
+func DefaultConnectionTracer(_ context.Context, p logging.Perspective, _ logging.ConnectionID) *logging.ConnectionTracer {
+	switch p {
+	case logging.PerspectiveClient:
+		defaultClientConnectionTracerOnce.Do(func() {
+			defaultClientConnectionTracer = NewClientConnectionTracerWithRegisterer(prometheus.DefaultRegisterer)
+		})
+		return defaultClientConnectionTracer
+	case logging.PerspectiveServer:
+		defaultServerConnectionTracerOnce.Do(func() {
+			defaultServerConnectionTracer = NewServerConnectionTracerWithRegisterer(prometheus.DefaultRegisterer)
+		})
+		return defaultServerConnectionTracer
+	default:
+		panic("invalid perspective")
 	}
 }
 
diff --git a/metrics/dashboards/README.md b/metrics/dashboards/README.md
index f7620421..cfb1001b 100644
--- a/metrics/dashboards/README.md
+++ b/metrics/dashboards/README.md
@@ -22,15 +22,11 @@ When using multiple `Transport`s, it is recommended to use the metrics tracer st
 
 Set a metrics connection tracer on the `Config`:
 ```go
-tracer := metrics.DefaultTracer()
 quic.Config{
-	Tracer: tracer,
+	Tracer: metrics.DefaultConnectionTracer,
 }
 ```
 
-It is recommended to use the same connection tracer returned by `DefaultTracer` on the `Config`s for all connections.
-
-
 Running:
 ```shell
 docker-compose up
diff --git a/qlog/qlog_dir.go b/qlog/qlog_dir.go
index e7acef4f..5ea5e3ae 100644
--- a/qlog/qlog_dir.go
+++ b/qlog/qlog_dir.go
@@ -13,9 +13,15 @@ import (
 )
 
 // DefaultTracer creates a qlog file in the qlog directory specified by the QLOGDIR environment variable.
+// Deprecated: use DefaultConnectionTracer instead.
+func DefaultTracer(ctx context.Context, p logging.Perspective, connID logging.ConnectionID) *logging.ConnectionTracer {
+	return DefaultConnectionTracer(ctx, p, connID)
+}
+
+// DefaultConnectionTracer creates a qlog file in the qlog directory specified by the QLOGDIR environment variable.
 // File names are <odcid>_<perspective>.qlog.
 // Returns nil if QLOGDIR is not set.
-func DefaultTracer(_ context.Context, p logging.Perspective, connID logging.ConnectionID) *logging.ConnectionTracer {
+func DefaultConnectionTracer(_ context.Context, p logging.Perspective, connID logging.ConnectionID) *logging.ConnectionTracer {
 	var label string
 	switch p {
 	case logging.PerspectiveClient:


Test patch:
diff --git a/integrationtests/self/qlog_dir_test.go b/integrationtests/self/qlog_dir_test.go
index 1af5a619..b81366e5 100644
--- a/integrationtests/self/qlog_dir_test.go
+++ b/integrationtests/self/qlog_dir_test.go
@@ -36,7 +36,7 @@ var _ = Describe("qlog dir tests", Serial, func() {
 			"localhost:0",
 			getTLSConfig(),
 			&quic.Config{
-				Tracer: qlog.DefaultTracer,
+				Tracer: qlog.DefaultConnectionTracer,
 			},
 		)
 		Expect(err).ToNot(HaveOccurred())
@@ -56,7 +56,7 @@ var _ = Describe("qlog dir tests", Serial, func() {
 			server.Addr().String(),
 			getTLSClientConfig(),
 			&quic.Config{
-				Tracer: qlog.DefaultTracer,
+				Tracer: qlog.DefaultConnectionTracer,
 			},
 		)
 		Expect(err).ToNot(HaveOccurred())
diff --git a/qlog/qlog_dir_test.go b/qlog/qlog_dir_test.go
index f0582b48..837465a5 100644
--- a/qlog/qlog_dir_test.go
+++ b/qlog/qlog_dir_test.go
@@ -36,7 +36,7 @@ var _ = Describe("qlog dir tests", Serial, func() {
 		qlogDir := path.Join(tempTestDirPath, "qlogs")
 		err := os.Setenv("QLOGDIR", qlogDir)
 		Expect(err).ToNot(HaveOccurred())
-		tracer := DefaultTracer(ctx, perspective, connID)
+		tracer := DefaultConnectionTracer(ctx, perspective, connID)
 		Expect(tracer).ToNot(BeNil())
 		tracer.Close()
 		_, err = os.Stat(qlogDir)
@@ -50,7 +50,7 @@ var _ = Describe("qlog dir tests", Serial, func() {
 	It("environment variable is not set", func() {
 		err := os.Setenv("QLOGDIR", "")
 		Expect(err).ToNot(HaveOccurred())
-		tracer := DefaultTracer(ctx, perspective, connID)
+		tracer := DefaultConnectionTracer(ctx, perspective, connID)
 		Expect(tracer).To(BeNil())
 	})
 })