Release 2024.12.2

## Summary
Some description...

Closes TUN-8822

CHANGES.md

@@ -1,3 +1,11 @@
+## 2024.12.2
+### New Features
+- This release introduces the ability to collect troubleshooting information from one instance of cloudflared running on the local machine. The command can be executed as `cloudflared tunnel diag`.
+
+## 2024.12.1
+### Notices
+- The use of the `--metrics` is still honoured meaning that if this flag is set the metrics server will try to bind it, however, this version includes a change that makes the metrics server bind to a port with a semi-deterministic approach. If the metrics flag is not present the server will bind to the first available port of the range 20241 to 20245. In case of all ports being unavailable then the fallback is to bind to a random port.
+
 ## 2024.10.0
 ### Bug Fixes
 - We fixed a bug related to `--grace-period`. Tunnels that use QUIC as transport weren't abiding by this waiting period before forcefully closing the connections to the edge. From now on, both QUIC and HTTP2 tunnels will wait for either the grace period to end (defaults to 30 seconds) or until the last in-flight request is handled. Users that wish to maintain the previous behavior should set `--grace-period` to 0 if `--protocol` is set to `quic`. This will force `cloudflared` to shutdown as soon as either SIGTERM or SIGINT is received.

RELEASE_NOTES

@@ -1,3 +1,18 @@
+2024.12.2
+- 2024-12-19 TUN-8822: Prevent concurrent usage of ICMPDecoder
+- 2024-12-18 TUN-8818: update changes document to reflect newly added diag subcommand
+- 2024-12-17 TUN-8817: Increase close session channel by one since there are two writers
+- 2024-12-13 TUN-8797: update CHANGES.md with note about semi-deterministic approach used to bind metrics server
+- 2024-12-13 TUN-8724: Add CLI command for diagnostic procedure
+- 2024-12-11 TUN-8786: calculate cli flags once for the diagnostic procedure
+- 2024-12-11 TUN-8792: Make diag/system endpoint always return a JSON
+- 2024-12-10 TUN-8783: fix log collectors for the diagnostic procedure
+- 2024-12-10 TUN-8785: include the icmp sources in the diag's tunnel state
+- 2024-12-10 TUN-8784: Set JSON encoder options to print formatted JSON when writing diag files
+
+2024.12.1
+- 2024-12-10 TUN-8795: update createrepo to createrepo_c to fix the release_pkgs.py script
+
 2024.12.0
 - 2024-12-09 TUN-8640: Add ICMP support for datagram V3
 - 2024-12-09 TUN-8789: make python package installation consistent

cfsetup.yaml

@@ -243,7 +243,7 @@ bullseye: &bullseye
       - python3-setuptools
       - python3-pip
       - reprepro
-      - createrepo
+      - createrepo-c
       - python3-venv
     post-cache:
       - python3 -m venv env

cmd/cloudflared/tunnel/cmd.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"net/url"
 	"os"
+	"path/filepath"
 	"runtime/trace"
 	"strings"
 	"sync"
@@ -235,6 +236,7 @@ func Commands() []*cli.Command {
 		buildDeleteCommand(),
 		buildCleanupCommand(),
 		buildTokenCommand(),
+		buildDiagCommand(),
 		// for compatibility, allow following as tunnel subcommands
 		proxydns.Command(true),
 		cliutil.RemovedCommand("db-connect"),
@@ -552,7 +554,15 @@ func StartServer(
 		tracker := tunnelstate.NewConnTracker(log)
 		observer.RegisterSink(tracker)
 
+		ipv4, ipv6, err := determineICMPSources(c, log)
+		sources := make([]string, 0)
+		if err == nil {
+			sources = append(sources, ipv4.String())
+			sources = append(sources, ipv6.String())
+		}
+
 		readinessServer := metrics.NewReadyServer(clientID, tracker)
+		cliFlags := nonSecretCliFlags(log, c, nonSecretFlagsList)
 		diagnosticHandler := diagnostic.NewDiagnosticHandler(
 			log,
 			0,
@@ -560,8 +570,8 @@ func StartServer(
 			tunnelConfig.NamedTunnel.Credentials.TunnelID,
 			clientID,
 			tracker,
-			c,
-			nonSecretFlagsList,
+			cliFlags,
+			sources,
 		)
 		metricsConfig := metrics.Config{
 			ReadyServer:         readinessServer,
@@ -1301,3 +1311,46 @@ reconnect [delay]
 		}
 	}
 }
+
+func nonSecretCliFlags(log *zerolog.Logger, cli *cli.Context, flagInclusionList []string) map[string]string {
+	flagsNames := cli.FlagNames()
+	flags := make(map[string]string, len(flagsNames))
+
+	for _, flag := range flagsNames {
+		value := cli.String(flag)
+
+		if value == "" {
+			continue
+		}
+
+		isIncluded := isFlagIncluded(flagInclusionList, flag)
+		if !isIncluded {
+			continue
+		}
+
+		switch flag {
+		case logger.LogDirectoryFlag, logger.LogFileFlag:
+			{
+				absolute, err := filepath.Abs(value)
+				if err != nil {
+					log.Error().Err(err).Msgf("could not convert %s path to absolute", flag)
+				} else {
+					flags[flag] = absolute
+				}
+			}
+		default:
+			flags[flag] = value
+		}
+	}
+	return flags
+}
+
+func isFlagIncluded(flagInclusionList []string, flag string) bool {
+	for _, include := range flagInclusionList {
+		if include == flag {
+			return true
+		}
+	}
+
+	return false
+}

cmd/cloudflared/tunnel/configuration.go

@@ -352,20 +352,9 @@ func adjustIPVersionByBindAddress(ipVersion allregions.ConfigIPVersion, ip net.I
 }
 
 func newICMPRouter(c *cli.Context, logger *zerolog.Logger) (ingress.ICMPRouterServer, error) {
-	ipv4Src, err := determineICMPv4Src(c.String("icmpv4-src"), logger)
+	ipv4Src, ipv6Src, err := determineICMPSources(c, logger)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to determine IPv4 source address for ICMP proxy")
-	}
-	logger.Info().Msgf("ICMP proxy will use %s as source for IPv4", ipv4Src)
-
-	ipv6Src, zone, err := determineICMPv6Src(c.String("icmpv6-src"), logger, ipv4Src)
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to determine IPv6 source address for ICMP proxy")
-	}
-	if zone != "" {
-		logger.Info().Msgf("ICMP proxy will use %s in zone %s as source for IPv6", ipv6Src, zone)
-	} else {
-		logger.Info().Msgf("ICMP proxy will use %s as source for IPv6", ipv6Src)
+		return nil, err
 	}
 
 	icmpRouter, err := ingress.NewICMPRouter(ipv4Src, ipv6Src, logger, icmpFunnelTimeout)
@@ -375,6 +364,28 @@ func newICMPRouter(c *cli.Context, logger *zerolog.Logger) (ingress.ICMPRouterSe
 	return icmpRouter, nil
 }
 
+func determineICMPSources(c *cli.Context, logger *zerolog.Logger) (netip.Addr, netip.Addr, error) {
+	ipv4Src, err := determineICMPv4Src(c.String("icmpv4-src"), logger)
+	if err != nil {
+		return netip.Addr{}, netip.Addr{}, errors.Wrap(err, "failed to determine IPv4 source address for ICMP proxy")
+	}
+
+	logger.Info().Msgf("ICMP proxy will use %s as source for IPv4", ipv4Src)
+
+	ipv6Src, zone, err := determineICMPv6Src(c.String("icmpv6-src"), logger, ipv4Src)
+	if err != nil {
+		return netip.Addr{}, netip.Addr{}, errors.Wrap(err, "failed to determine IPv6 source address for ICMP proxy")
+	}
+
+	if zone != "" {
+		logger.Info().Msgf("ICMP proxy will use %s in zone %s as source for IPv6", ipv6Src, zone)
+	} else {
+		logger.Info().Msgf("ICMP proxy will use %s as source for IPv6", ipv6Src)
+	}
+
+	return ipv4Src, ipv6Src, nil
+}
+
 func determineICMPv4Src(userDefinedSrc string, logger *zerolog.Logger) (netip.Addr, error) {
 	if userDefinedSrc != "" {
 		addr, err := netip.ParseAddr(userDefinedSrc)

cmd/cloudflared/tunnel/subcommands.go

@@ -28,16 +28,26 @@ import (
 	"github.com/cloudflare/cloudflared/cmd/cloudflared/updater"
 	"github.com/cloudflare/cloudflared/config"
 	"github.com/cloudflare/cloudflared/connection"
+	"github.com/cloudflare/cloudflared/diagnostic"
+	"github.com/cloudflare/cloudflared/metrics"
 )
 
 const (
-	allSortByOptions     = "name, id, createdAt, deletedAt, numConnections"
-	connsSortByOptions   = "id, startedAt, numConnections, version"
-	CredFileFlagAlias    = "cred-file"
-	CredFileFlag         = "credentials-file"
-	CredContentsFlag     = "credentials-contents"
-	TunnelTokenFlag      = "token"
-	overwriteDNSFlagName = "overwrite-dns"
+	allSortByOptions        = "name, id, createdAt, deletedAt, numConnections"
+	connsSortByOptions      = "id, startedAt, numConnections, version"
+	CredFileFlagAlias       = "cred-file"
+	CredFileFlag            = "credentials-file"
+	CredContentsFlag        = "credentials-contents"
+	TunnelTokenFlag         = "token"
+	overwriteDNSFlagName    = "overwrite-dns"
+	noDiagLogsFlagName      = "no-diag-logs"
+	noDiagMetricsFlagName   = "no-diag-metrics"
+	noDiagSystemFlagName    = "no-diag-system"
+	noDiagRuntimeFlagName   = "no-diag-runtime"
+	noDiagNetworkFlagName   = "no-diag-network"
+	diagContainerIDFlagName = "diag-container-id"
+	diagPodFlagName         = "diag-pod-id"
+	metricsFlagName         = "metrics"
 
 	LogFieldTunnelID = "tunnelID"
 )
@@ -179,6 +189,46 @@ var (
 		Usage:   "Source address and the interface name to send/receive ICMPv6 messages. If not provided cloudflared will dial a local address to determine the source IP or fallback to ::.",
 		EnvVars: []string{"TUNNEL_ICMPV6_SRC"},
 	}
+	metricsFlag = &cli.StringFlag{
+		Name:  metricsFlagName,
+		Usage: "The metrics server address i.e.: 127.0.0.1:12345. If your instance is running in a Docker/Kubernetes environment you need to setup port forwarding for your application.",
+		Value: "",
+	}
+	diagContainerFlag = &cli.StringFlag{
+		Name:  diagContainerIDFlagName,
+		Usage: "Container ID or Name to collect logs from",
+		Value: "",
+	}
+	diagPodFlag = &cli.StringFlag{
+		Name:  diagPodFlagName,
+		Usage: "Kubernetes POD to collect logs from",
+		Value: "",
+	}
+	noDiagLogsFlag = &cli.BoolFlag{
+		Name:  noDiagLogsFlagName,
+		Usage: "Log collection will not be performed",
+		Value: false,
+	}
+	noDiagMetricsFlag = &cli.BoolFlag{
+		Name:  noDiagMetricsFlagName,
+		Usage: "Metric collection will not be performed",
+		Value: false,
+	}
+	noDiagSystemFlag = &cli.BoolFlag{
+		Name:  noDiagSystemFlagName,
+		Usage: "System information collection will not be performed",
+		Value: false,
+	}
+	noDiagRuntimeFlag = &cli.BoolFlag{
+		Name:  noDiagRuntimeFlagName,
+		Usage: "Runtime information collection will not be performed",
+		Value: false,
+	}
+	noDiagNetworkFlag = &cli.BoolFlag{
+		Name:  noDiagNetworkFlagName,
+		Usage: "Network diagnostics won't be performed",
+		Value: false,
+	}
 )
 
 func buildCreateCommand() *cli.Command {
@@ -375,7 +425,6 @@ func formatAndPrintTunnelList(tunnels []*cfapi.Tunnel, showRecentlyDisconnected
 }
 
 func fmtConnections(connections []cfapi.Connection, showRecentlyDisconnected bool) string {
-
 	// Count connections per colo
 	numConnsPerColo := make(map[string]uint, len(connections))
 	for _, connection := range connections {
@@ -897,8 +946,10 @@ func lbRouteFromArg(c *cli.Context) (cfapi.HostnameRoute, error) {
 	return cfapi.NewLBRoute(lbName, lbPool), nil
 }
 
-var nameRegex = regexp.MustCompile("^[_a-zA-Z0-9][-_.a-zA-Z0-9]*$")
-var hostNameRegex = regexp.MustCompile("^[*_a-zA-Z0-9][-_.a-zA-Z0-9]*$")
+var (
+	nameRegex     = regexp.MustCompile("^[_a-zA-Z0-9][-_.a-zA-Z0-9]*$")
+	hostNameRegex = regexp.MustCompile("^[*_a-zA-Z0-9][-_.a-zA-Z0-9]*$")
+)
 
 func validateName(s string, allowWildcardSubdomain bool) bool {
 	if allowWildcardSubdomain {
@@ -986,3 +1037,78 @@ SUBCOMMAND OPTIONS:
 `
 	return fmt.Sprintf(template, parentFlagsHelp)
 }
+
+func buildDiagCommand() *cli.Command {
+	return &cli.Command{
+		Name:        "diag",
+		Action:      cliutil.ConfiguredAction(diagCommand),
+		Usage:       "Creates a diagnostic report from a local cloudflared instance",
+		UsageText:   "cloudflared tunnel [tunnel command options] diag [subcommand options]",
+		Description: "cloudflared tunnel diag will create a diagnostic report of a local cloudflared instance. The diagnostic procedure collects: logs, metrics, system information, traceroute to Cloudflare Edge, and runtime information. Since there may be multiple instances of cloudflared running the --metrics option may be provided to target a specific instance.",
+		Flags: []cli.Flag{
+			metricsFlag,
+			diagContainerFlag,
+			diagPodFlag,
+			noDiagLogsFlag,
+			noDiagMetricsFlag,
+			noDiagSystemFlag,
+			noDiagRuntimeFlag,
+			noDiagNetworkFlag,
+		},
+		CustomHelpTemplate: commandHelpTemplate(),
+	}
+}
+
+func diagCommand(ctx *cli.Context) error {
+	sctx, err := newSubcommandContext(ctx)
+	if err != nil {
+		return err
+	}
+	log := sctx.log
+	options := diagnostic.Options{
+		KnownAddresses: metrics.GetMetricsKnownAddresses(metrics.Runtime),
+		Address:        sctx.c.String(metricsFlagName),
+		ContainerID:    sctx.c.String(diagContainerIDFlagName),
+		PodID:          sctx.c.String(diagPodFlagName),
+		Toggles: diagnostic.Toggles{
+			NoDiagLogs:    sctx.c.Bool(noDiagLogsFlagName),
+			NoDiagMetrics: sctx.c.Bool(noDiagMetricsFlagName),
+			NoDiagSystem:  sctx.c.Bool(noDiagSystemFlagName),
+			NoDiagRuntime: sctx.c.Bool(noDiagRuntimeFlagName),
+			NoDiagNetwork: sctx.c.Bool(noDiagNetworkFlagName),
+		},
+	}
+
+	if options.Address == "" {
+		log.Info().Msg("If your instance is running in a Docker/Kubernetes environment you need to setup port forwarding for your application.")
+	}
+
+	states, err := diagnostic.RunDiagnostic(log, options)
+
+	if errors.Is(err, diagnostic.ErrMetricsServerNotFound) {
+		log.Warn().Msg("No instances found")
+		return nil
+	}
+	if errors.Is(err, diagnostic.ErrMultipleMetricsServerFound) {
+		if states != nil {
+			log.Info().Msgf("Found multiple instances running:")
+			for _, state := range states {
+				log.Info().Msgf("Instance: tunnel-id=%s connector-id=%s metrics-address=%s", state.TunnelID, state.ConnectorID, state.URL.String())
+			}
+			log.Info().Msgf("To select one instance use the option --metrics")
+		}
+		return nil
+	}
+
+	if errors.Is(err, diagnostic.ErrLogConfigurationIsInvalid) {
+		log.Info().Msg("Couldn't extract logs from the instance. If the instance is running in a containerized environment use the option --diag-container-id or --diag-pod-id. If there is no logging configuration use --no-diag-logs.")
+	}
+
+	if err != nil {
+		log.Warn().Msg("Diagnostic completed with one or more errors")
+	} else {
+		log.Info().Msg("Diagnostic completed")
+	}
+
+	return nil
+}

diagnostic/client.go

@@ -141,7 +141,7 @@ func (client *httpClient) GetSystemInformation(ctx context.Context, writer io.Wr
 		return err
 	}
 
-	return copyToWriter(response, writer)
+	return copyJSONToWriter(response, writer)
 }
 
 func (client *httpClient) GetMetrics(ctx context.Context, writer io.Writer) error {
@@ -159,7 +159,7 @@ func (client *httpClient) GetTunnelConfiguration(ctx context.Context, writer io.
 		return err
 	}
 
-	return copyToWriter(response, writer)
+	return copyJSONToWriter(response, writer)
 }
 
 func (client *httpClient) GetCliConfiguration(ctx context.Context, writer io.Writer) error {
@@ -168,7 +168,7 @@ func (client *httpClient) GetCliConfiguration(ctx context.Context, writer io.Wri
 		return err
 	}
 
-	return copyToWriter(response, writer)
+	return copyJSONToWriter(response, writer)
 }
 
 func copyToWriter(response *http.Response, writer io.Writer) error {
@@ -176,7 +176,29 @@ func copyToWriter(response *http.Response, writer io.Writer) error {
 
 	_, err := io.Copy(writer, response.Body)
 	if err != nil {
-		return fmt.Errorf("error writing metrics: %w", err)
+		return fmt.Errorf("error writing response: %w", err)
+	}
+
+	return nil
+}
+
+func copyJSONToWriter(response *http.Response, writer io.Writer) error {
+	defer response.Body.Close()
+
+	var data interface{}
+
+	decoder := json.NewDecoder(response.Body)
+
+	err := decoder.Decode(&data)
+	if err != nil {
+		return fmt.Errorf("diagnostic client error whilst reading response: %w", err)
+	}
+
+	encoder := newFormattedEncoder(writer)
+
+	err = encoder.Encode(data)
+	if err != nil {
+		return fmt.Errorf("diagnostic client error whilst writing json: %w", err)
 	}
 
 	return nil

diagnostic/diagnostic.go

@@ -9,6 +9,7 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"strings"
 	"sync"
 	"time"
 
@@ -40,6 +41,17 @@ type taskResult struct {
 	path   string
 }
 
+func (result taskResult) MarshalJSON() ([]byte, error) {
+	s := map[string]string{
+		"result": result.Result,
+	}
+	if result.Err != nil {
+		s["error"] = result.Err.Error()
+	}
+
+	return json.Marshal(s)
+}
+
 // Struct used to hold the results of different routines executing the network collection.
 type networkCollectionResult struct {
 	name string
@@ -151,17 +163,7 @@ func collectNetworkResultRoutine(
 	}
 
 	hops, raw, err := collector.Collect(ctx, network.NewTraceOptions(hopsNo, timeout, hostname, useIPv4))
-	if err != nil {
-		if raw == "" {
-			// An error happened and there is no raw output
-			results <- networkCollectionResult{name, nil, "", err}
-		} else {
-			// An error happened and there is raw output then write to file
-			results <- networkCollectionResult{name, nil, raw, nil}
-		}
-	} else {
-		results <- networkCollectionResult{name, hops, raw, nil}
-	}
+	results <- networkCollectionResult{name, hops, raw, err}
 }
 
 func gatherNetworkInformation(ctx context.Context) map[string]networkCollectionResult {
@@ -198,10 +200,6 @@ func gatherNetworkInformation(ctx context.Context) map[string]networkCollectionR
 
 	for range len(hostAndIPversionPairs) {
 		result := <-results
-		if result.err != nil {
-			continue
-		}
-
 		resultMap[result.name] = result
 	}
 
@@ -238,22 +236,30 @@ func rawNetworkInformationWriter(resultMap map[string]networkCollectionResult) (
 
 	defer networkDumpHandle.Close()
 
+	var exitErr error
+
 	for k, v := range resultMap {
-		_, err := networkDumpHandle.WriteString(k + "\n" + v.raw + "\n")
-		if err != nil {
-			return "", fmt.Errorf("error writing raw network information: %w", err)
+		if v.err != nil {
+			if exitErr == nil {
+				exitErr = v.err
+			}
+
+			_, err := networkDumpHandle.WriteString(k + "\nno content\n")
+			if err != nil {
+				return networkDumpHandle.Name(), fmt.Errorf("error writing 'no content' to raw network file: %w", err)
+			}
+		} else {
+			_, err := networkDumpHandle.WriteString(k + "\n" + v.raw + "\n")
+			if err != nil {
+				return networkDumpHandle.Name(), fmt.Errorf("error writing raw network information: %w", err)
+			}
 		}
 	}
 
-	return networkDumpHandle.Name(), nil
+	return networkDumpHandle.Name(), exitErr
 }
 
 func jsonNetworkInformationWriter(resultMap map[string]networkCollectionResult) (string, error) {
-	jsonMap := make(map[string][]*network.Hop, len(resultMap))
-	for k, v := range resultMap {
-		jsonMap[k] = v.info
-	}
-
 	networkDumpHandle, err := os.Create(filepath.Join(os.TempDir(), networkBaseName))
 	if err != nil {
 		return "", ErrCreatingTemporaryFile
@@ -261,12 +267,25 @@ func jsonNetworkInformationWriter(resultMap map[string]networkCollectionResult)
 
 	defer networkDumpHandle.Close()
 
-	err = json.NewEncoder(networkDumpHandle).Encode(jsonMap)
-	if err != nil {
-		return "", fmt.Errorf("error encoding network information results: %w", err)
+	encoder := newFormattedEncoder(networkDumpHandle)
+
+	var exitErr error
+
+	jsonMap := make(map[string][]*network.Hop, len(resultMap))
+	for k, v := range resultMap {
+		jsonMap[k] = v.info
+
+		if exitErr == nil && v.err != nil {
+			exitErr = v.err
+		}
 	}
 
-	return networkDumpHandle.Name(), nil
+	err = encoder.Encode(jsonMap)
+	if err != nil {
+		return networkDumpHandle.Name(), fmt.Errorf("error encoding network information results: %w", err)
+	}
+
+	return networkDumpHandle.Name(), exitErr
 }
 
 func collectFromEndpointAdapter(collect collectToWriterFunc, fileName string) collectFunc {
@@ -279,7 +298,7 @@ func collectFromEndpointAdapter(collect collectToWriterFunc, fileName string) co
 
 		err = collect(ctx, dumpHandle)
 		if err != nil {
-			return "", ErrCreatingTemporaryFile
+			return dumpHandle.Name(), fmt.Errorf("error running collector: %w", err)
 		}
 
 		return dumpHandle.Name(), nil
@@ -300,11 +319,14 @@ func tunnelStateCollectEndpointAdapter(client HTTPClient, tunnel *TunnelState, f
 			tunnel = tunnelResponse
 		}
 
-		encoder := json.NewEncoder(writer)
+		encoder := newFormattedEncoder(writer)
 
 		err := encoder.Encode(tunnel)
+		if err != nil {
+			return fmt.Errorf("error encoding tunnel state: %w", err)
+		}
 
-		return fmt.Errorf("error encoding tunnel state: %w", err)
+		return nil
 	}
 
 	return collectFromEndpointAdapter(endpointFunc, fileName)
@@ -324,15 +346,14 @@ func resolveInstanceBaseURL(
 	addresses []string,
 ) (*url.URL, *TunnelState, []*AddressableTunnelState, error) {
 	if metricsServerAddress != "" {
+		if !strings.HasPrefix(metricsServerAddress, "http://") {
+			metricsServerAddress = "http://" + metricsServerAddress
+		}
 		url, err := url.Parse(metricsServerAddress)
 		if err != nil {
 			return nil, nil, nil, fmt.Errorf("provided address is not valid: %w", err)
 		}
 
-		if url.Scheme == "" {
-			url.Scheme = "http://"
-		}
-
 		return url, nil, nil, nil
 	}
 
@@ -421,7 +442,9 @@ func createTaskReport(taskReport map[string]taskResult) (string, error) {
 	}
 	defer dumpHandle.Close()
 
-	err = json.NewEncoder(dumpHandle).Encode(taskReport)
+	encoder := newFormattedEncoder(dumpHandle)
+
+	err = encoder.Encode(taskReport)
 	if err != nil {
 		return "", fmt.Errorf("error encoding task results: %w", err)
 	}
@@ -511,9 +534,15 @@ func RunDiagnostic(
 	jobsReport := runJobs(ctx, jobs, log)
 	paths := make([]string, 0)
 
+	var gerr error
+
 	for _, v := range jobsReport {
 		paths = append(paths, v.path)
 
+		if gerr == nil && v.Err != nil {
+			gerr = v.Err
+		}
+
 		defer func() {
 			if !errors.Is(v.Err, ErrCreatingTemporaryFile) {
 				os.Remove(v.path)
@@ -523,14 +552,10 @@ func RunDiagnostic(
 
 	zipfile, err := CreateDiagnosticZipFile(zipName, paths)
 	if err != nil {
-		if zipfile != "" {
-			os.Remove(zipfile)
-		}
-
 		return nil, err
 	}
 
 	log.Info().Msgf("Diagnostic file written: %v", zipfile)
 
-	return nil, nil
+	return nil, gerr
 }

diagnostic/diagnostic_utils.go

@@ -3,6 +3,7 @@ package diagnostic
 import (
 	"archive/zip"
 	"context"
+	"encoding/json"
 	"fmt"
 	"io"
 	"net/url"
@@ -138,3 +139,10 @@ func FindMetricsServer(
 
 	return nil, instances, ErrMultipleMetricsServerFound
 }
+
+// newFormattedEncoder return a JSON encoder with identation
+func newFormattedEncoder(w io.Writer) *json.Encoder {
+	encoder := json.NewEncoder(w)
+	encoder.SetIndent("", "  ")
+	return encoder
+}

diagnostic/diagnostic_utils_test.go

@@ -25,7 +25,7 @@ func helperCreateServer(t *testing.T, listeners *gracenet.Net, tunnelID uuid.UUI
 	require.NoError(t, err)
 	log := zerolog.Nop()
 	tracker := tunnelstate.NewConnTracker(&log)
-	handler := diagnostic.NewDiagnosticHandler(&log, 0, nil, tunnelID, connectorID, tracker, nil, []string{})
+	handler := diagnostic.NewDiagnosticHandler(&log, 0, nil, tunnelID, connectorID, tracker, map[string]string{}, []string{})
 	router := http.NewServeMux()
 	router.HandleFunc("/diag/tunnel", handler.TunnelStateHandler)
 	server := &http.Server{

diagnostic/handlers.go

@@ -5,27 +5,24 @@ import (
 	"encoding/json"
 	"net/http"
 	"os"
-	"path/filepath"
 	"strconv"
 	"time"
 
 	"github.com/google/uuid"
 	"github.com/rs/zerolog"
-	"github.com/urfave/cli/v2"
 
-	"github.com/cloudflare/cloudflared/logger"
 	"github.com/cloudflare/cloudflared/tunnelstate"
 )
 
 type Handler struct {
-	log               *zerolog.Logger
-	timeout           time.Duration
-	systemCollector   SystemCollector
-	tunnelID          uuid.UUID
-	connectorID       uuid.UUID
-	tracker           *tunnelstate.ConnTracker
-	cli               *cli.Context
-	flagInclusionList []string
+	log             *zerolog.Logger
+	timeout         time.Duration
+	systemCollector SystemCollector
+	tunnelID        uuid.UUID
+	connectorID     uuid.UUID
+	tracker         *tunnelstate.ConnTracker
+	cliFlags        map[string]string
+	icmpSources     []string
 }
 
 func NewDiagnosticHandler(
@@ -35,23 +32,24 @@ func NewDiagnosticHandler(
 	tunnelID uuid.UUID,
 	connectorID uuid.UUID,
 	tracker *tunnelstate.ConnTracker,
-	cli *cli.Context,
-	flagInclusionList []string,
+	cliFlags map[string]string,
+	icmpSources []string,
 ) *Handler {
 	logger := log.With().Logger()
 	if timeout == 0 {
 		timeout = defaultCollectorTimeout
 	}
 
+	cliFlags[configurationKeyUID] = strconv.Itoa(os.Getuid())
 	return &Handler{
-		log:               &logger,
-		timeout:           timeout,
-		systemCollector:   systemCollector,
-		tunnelID:          tunnelID,
-		connectorID:       connectorID,
-		tracker:           tracker,
-		cli:               cli,
-		flagInclusionList: flagInclusionList,
+		log:             &logger,
+		timeout:         timeout,
+		systemCollector: systemCollector,
+		tunnelID:        tunnelID,
+		connectorID:     connectorID,
+		tracker:         tracker,
+		cliFlags:        cliFlags,
+		icmpSources:     icmpSources,
 	}
 }
 
@@ -61,6 +59,11 @@ func (handler *Handler) InstallEndpoints(router *http.ServeMux) {
 	router.HandleFunc(systemInformationEndpoint, handler.SystemHandler)
 }
 
+type SystemInformationResponse struct {
+	Info *SystemInformation `json:"info"`
+	Err  error              `json:"errors"`
+}
+
 func (handler *Handler) SystemHandler(writer http.ResponseWriter, request *http.Request) {
 	logger := handler.log.With().Str(collectorField, systemCollectorName).Logger()
 	logger.Info().Msg("Collection started")
@@ -71,30 +74,15 @@ func (handler *Handler) SystemHandler(writer http.ResponseWriter, request *http.
 
 	defer cancel()
 
-	info, rawInfo, err := handler.systemCollector.Collect(ctx)
-	if err != nil {
-		logger.Error().Err(err).Msg("error occurred whilst collecting system information")
+	info, err := handler.systemCollector.Collect(ctx)
 
-		if rawInfo != "" {
-			logger.Info().Msg("using raw information fallback")
-			bytes := []byte(rawInfo)
-			writeResponse(writer, bytes, &logger)
-		} else {
-			logger.Error().Msg("no raw information available")
-			writer.WriteHeader(http.StatusInternalServerError)
-		}
-
-		return
-	}
-
-	if info == nil {
-		logger.Error().Msgf("system information collection is nil")
-		writer.WriteHeader(http.StatusInternalServerError)
+	response := SystemInformationResponse{
+		Info: info,
+		Err:  err,
 	}
 
 	encoder := json.NewEncoder(writer)
-
-	err = encoder.Encode(info)
+	err = encoder.Encode(response)
 	if err != nil {
 		logger.Error().Err(err).Msgf("error occurred whilst serializing information")
 		writer.WriteHeader(http.StatusInternalServerError)
@@ -105,6 +93,7 @@ type TunnelState struct {
 	TunnelID    uuid.UUID                           `json:"tunnelID,omitempty"`
 	ConnectorID uuid.UUID                           `json:"connectorID,omitempty"`
 	Connections []tunnelstate.IndexedConnectionInfo `json:"connections,omitempty"`
+	ICMPSources []string                            `json:"icmp_sources,omitempty"`
 }
 
 func (handler *Handler) TunnelStateHandler(writer http.ResponseWriter, _ *http.Request) {
@@ -117,6 +106,7 @@ func (handler *Handler) TunnelStateHandler(writer http.ResponseWriter, _ *http.R
 		handler.tunnelID,
 		handler.connectorID,
 		handler.tracker.GetActiveConnections(),
+		handler.icmpSources,
 	}
 	encoder := json.NewEncoder(writer)
 
@@ -135,68 +125,15 @@ func (handler *Handler) ConfigurationHandler(writer http.ResponseWriter, _ *http
 		log.Info().Msg("Collection finished")
 	}()
 
-	flagsNames := handler.cli.FlagNames()
-	flags := make(map[string]string, len(flagsNames))
-
-	for _, flag := range flagsNames {
-		value := handler.cli.String(flag)
-
-		// empty values are not relevant
-		if value == "" {
-			continue
-		}
-
-		// exclude flags that are sensitive
-		isIncluded := handler.isFlagIncluded(flag)
-		if !isIncluded {
-			continue
-		}
-
-		switch flag {
-		case logger.LogDirectoryFlag:
-			fallthrough
-		case logger.LogFileFlag:
-			{
-				// the log directory may be relative to the instance thus it must be resolved
-				absolute, err := filepath.Abs(value)
-				if err != nil {
-					handler.log.Error().Err(err).Msgf("could not convert %s path to absolute", flag)
-				} else {
-					flags[flag] = absolute
-				}
-			}
-		default:
-			flags[flag] = value
-		}
-	}
-
-	// The UID is included to help the
-	// diagnostic tool to understand
-	// if this instance is managed or not.
-	flags[configurationKeyUID] = strconv.Itoa(os.Getuid())
 	encoder := json.NewEncoder(writer)
 
-	err := encoder.Encode(flags)
+	err := encoder.Encode(handler.cliFlags)
 	if err != nil {
 		handler.log.Error().Err(err).Msgf("error occurred whilst serializing response")
 		writer.WriteHeader(http.StatusInternalServerError)
 	}
 }
 
-func (handler *Handler) isFlagIncluded(flag string) bool {
-	isIncluded := false
-
-	for _, include := range handler.flagInclusionList {
-		if include == flag {
-			isIncluded = true
-
-			break
-		}
-	}
-
-	return isIncluded
-}
-
 func writeResponse(w http.ResponseWriter, bytes []byte, logger *zerolog.Logger) {
 	bytesWritten, err := w.Write(bytes)
 	if err != nil {

diagnostic/handlers_test.go

@@ -4,47 +4,32 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
-	"flag"
-	"io"
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"runtime"
 	"testing"
 
 	"github.com/google/uuid"
 	"github.com/rs/zerolog"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"github.com/urfave/cli/v2"
 
 	"github.com/cloudflare/cloudflared/connection"
 	"github.com/cloudflare/cloudflared/diagnostic"
 	"github.com/cloudflare/cloudflared/tunnelstate"
 )
 
-type SystemCollectorMock struct{}
+type SystemCollectorMock struct {
+	systemInfo *diagnostic.SystemInformation
+	err        error
+}
 
 const (
 	systemInformationKey = "sikey"
-	rawInformationKey    = "rikey"
 	errorKey             = "errkey"
 )
 
-func buildCliContext(t *testing.T, flags map[string]string) *cli.Context {
-	t.Helper()
-
-	flagSet := flag.NewFlagSet("", flag.PanicOnError)
-	ctx := cli.NewContext(cli.NewApp(), flagSet, nil)
-
-	for k, v := range flags {
-		flagSet.String(k, v, "")
-		err := ctx.Set(k, v)
-		require.NoError(t, err)
-	}
-
-	return ctx
-}
-
 func newTrackerFromConns(t *testing.T, connections []tunnelstate.IndexedConnectionInfo) *tunnelstate.ConnTracker {
 	t.Helper()
 
@@ -63,25 +48,8 @@ func newTrackerFromConns(t *testing.T, connections []tunnelstate.IndexedConnecti
 	return tracker
 }
 
-func setCtxValuesForSystemCollector(
-	systemInfo *diagnostic.SystemInformation,
-	rawInfo string,
-	err error,
-) context.Context {
-	ctx := context.Background()
-	ctx = context.WithValue(ctx, systemInformationKey, systemInfo)
-	ctx = context.WithValue(ctx, rawInformationKey, rawInfo)
-	ctx = context.WithValue(ctx, errorKey, err)
-
-	return ctx
-}
-
-func (*SystemCollectorMock) Collect(ctx context.Context) (*diagnostic.SystemInformation, string, error) {
-	si, _ := ctx.Value(systemInformationKey).(*diagnostic.SystemInformation)
-	ri, _ := ctx.Value(rawInformationKey).(string)
-	err, _ := ctx.Value(errorKey).(error)
-
-	return si, ri, err
+func (collector *SystemCollectorMock) Collect(context.Context) (*diagnostic.SystemInformation, error) {
+	return collector.systemInfo, collector.err
 }
 
 func TestSystemHandler(t *testing.T) {
@@ -91,7 +59,6 @@ func TestSystemHandler(t *testing.T) {
 	tests := []struct {
 		name       string
 		systemInfo *diagnostic.SystemInformation
-		rawInfo    string
 		err        error
 		statusCode int
 	}{
@@ -100,48 +67,39 @@ func TestSystemHandler(t *testing.T) {
 			systemInfo: diagnostic.NewSystemInformation(
 				0, 0, 0, 0,
 				"string", "string", "string", "string",
-				"string", "string", nil,
+				"string", "string",
+				runtime.Version(), runtime.GOARCH, nil,
 			),
-			rawInfo:    "",
+
 			err:        nil,
 			statusCode: http.StatusOK,
 		},
-		{
-			name: "on error and raw info", systemInfo: nil,
-			rawInfo: "raw info", err: errors.New("an error"), statusCode: http.StatusOK,
-		},
 		{
 			name: "on error and no raw info", systemInfo: nil,
-			rawInfo: "", err: errors.New("an error"), statusCode: http.StatusInternalServerError,
-		},
-		{
-			name: "malformed response", systemInfo: nil, rawInfo: "", err: nil, statusCode: http.StatusInternalServerError,
+			err: errors.New("an error"), statusCode: http.StatusOK,
 		},
 	}
 
 	for _, tCase := range tests {
 		t.Run(tCase.name, func(t *testing.T) {
 			t.Parallel()
-
-			handler := diagnostic.NewDiagnosticHandler(&log, 0, &SystemCollectorMock{}, uuid.New(), uuid.New(), nil, nil, nil)
+			handler := diagnostic.NewDiagnosticHandler(&log, 0, &SystemCollectorMock{
+				systemInfo: tCase.systemInfo,
+				err:        tCase.err,
+			}, uuid.New(), uuid.New(), nil, map[string]string{}, nil)
 			recorder := httptest.NewRecorder()
-			ctx := setCtxValuesForSystemCollector(tCase.systemInfo, tCase.rawInfo, tCase.err)
-			request, err := http.NewRequestWithContext(ctx, http.MethodGet, "/diag/syste,", nil)
+			ctx := context.Background()
+			request, err := http.NewRequestWithContext(ctx, http.MethodGet, "/diag/system", nil)
 			require.NoError(t, err)
 			handler.SystemHandler(recorder, request)
 
 			assert.Equal(t, tCase.statusCode, recorder.Code)
 			if tCase.statusCode == http.StatusOK && tCase.systemInfo != nil {
-				var response diagnostic.SystemInformation
-
+				var response diagnostic.SystemInformationResponse
 				decoder := json.NewDecoder(recorder.Body)
-				err = decoder.Decode(&response)
+				err := decoder.Decode(&response)
 				require.NoError(t, err)
-				assert.Equal(t, tCase.systemInfo, &response)
-			} else if tCase.statusCode == http.StatusOK && tCase.rawInfo != "" {
-				rawBytes, err := io.ReadAll(recorder.Body)
-				require.NoError(t, err)
-				assert.Equal(t, tCase.rawInfo, string(rawBytes))
+				assert.Equal(t, tCase.systemInfo, response.Info)
 			}
 		})
 	}
@@ -156,6 +114,7 @@ func TestTunnelStateHandler(t *testing.T) {
 		tunnelID    uuid.UUID
 		clientID    uuid.UUID
 		connections []tunnelstate.IndexedConnectionInfo
+		icmpSources []string
 	}{
 		{
 			name:     "case1",
@@ -163,9 +122,10 @@ func TestTunnelStateHandler(t *testing.T) {
 			clientID: uuid.New(),
 		},
 		{
-			name:     "case2",
-			tunnelID: uuid.New(),
-			clientID: uuid.New(),
+			name:        "case2",
+			tunnelID:    uuid.New(),
+			clientID:    uuid.New(),
+			icmpSources: []string{"172.17.0.3", "::1"},
 			connections: []tunnelstate.IndexedConnectionInfo{{
 				ConnectionInfo: tunnelstate.ConnectionInfo{
 					IsConnected: true,
@@ -181,7 +141,16 @@ func TestTunnelStateHandler(t *testing.T) {
 		t.Run(tCase.name, func(t *testing.T) {
 			t.Parallel()
 			tracker := newTrackerFromConns(t, tCase.connections)
-			handler := diagnostic.NewDiagnosticHandler(&log, 0, nil, tCase.tunnelID, tCase.clientID, tracker, nil, nil)
+			handler := diagnostic.NewDiagnosticHandler(
+				&log,
+				0,
+				nil,
+				tCase.tunnelID,
+				tCase.clientID,
+				tracker,
+				map[string]string{},
+				tCase.icmpSources,
+			)
 			recorder := httptest.NewRecorder()
 			handler.TunnelStateHandler(recorder, nil)
 			decoder := json.NewDecoder(recorder.Body)
@@ -193,6 +162,7 @@ func TestTunnelStateHandler(t *testing.T) {
 			assert.Equal(t, tCase.tunnelID, response.TunnelID)
 			assert.Equal(t, tCase.clientID, response.ConnectorID)
 			assert.Equal(t, tCase.connections, response.Connections)
+			assert.Equal(t, tCase.icmpSources, response.ICMPSources)
 		})
 	}
 }
@@ -217,10 +187,10 @@ func TestConfigurationHandler(t *testing.T) {
 		{
 			name: "cli with flags",
 			flags: map[string]string{
-				"a": "a",
-				"b": "a",
-				"c": "a",
-				"d": "a",
+				"b":   "a",
+				"c":   "a",
+				"d":   "a",
+				"uid": "0",
 			},
 			expected: map[string]string{
 				"b":   "a",
@@ -233,11 +203,11 @@ func TestConfigurationHandler(t *testing.T) {
 
 	for _, tCase := range tests {
 		t.Run(tCase.name, func(t *testing.T) {
+			t.Parallel()
+
 			var response map[string]string
 
-			t.Parallel()
-			ctx := buildCliContext(t, tCase.flags)
-			handler := diagnostic.NewDiagnosticHandler(&log, 0, nil, uuid.New(), uuid.New(), nil, ctx, []string{"b", "c", "d"})
+			handler := diagnostic.NewDiagnosticHandler(&log, 0, nil, uuid.New(), uuid.New(), nil, tCase.flags, nil)
 			recorder := httptest.NewRecorder()
 			handler.ConfigurationHandler(recorder, nil)
 			decoder := json.NewDecoder(recorder.Body)

diagnostic/log_collector_host.go

@@ -4,13 +4,16 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"runtime"
 )
 
 const (
-	linuxManagedLogsPath  = "/var/log/cloudflared.err"
-	darwinManagedLogsPath = "/Library/Logs/com.cloudflare.cloudflared.err.log"
+	linuxManagedLogsPath          = "/var/log/cloudflared.err"
+	darwinManagedLogsPath         = "/Library/Logs/com.cloudflare.cloudflared.err.log"
+	linuxServiceConfigurationPath = "/etc/systemd/system/cloudflared.service"
+	linuxSystemdPath              = "/run/systemd/system"
 )
 
 type HostLogCollector struct {
@@ -23,6 +26,28 @@ func NewHostLogCollector(client HTTPClient) *HostLogCollector {
 	}
 }
 
+func extractLogsFromJournalCtl(ctx context.Context) (*LogInformation, error) {
+	tmp := os.TempDir()
+
+	outputHandle, err := os.Create(filepath.Join(tmp, logFilename))
+	if err != nil {
+		return nil, fmt.Errorf("error opening output file: %w", err)
+	}
+
+	defer outputHandle.Close()
+
+	command := exec.CommandContext(
+		ctx,
+		"journalctl",
+		"--since",
+		"2 weeks ago",
+		"-u",
+		"cloudflared.service",
+	)
+
+	return PipeCommandOutputToFile(command, outputHandle)
+}
+
 func getServiceLogPath() (string, error) {
 	switch runtime.GOOS {
 	case "darwin":
@@ -55,6 +80,13 @@ func (collector *HostLogCollector) Collect(ctx context.Context) (*LogInformation
 	}
 
 	if logConfiguration.uid == 0 {
+		_, statSystemdErr := os.Stat(linuxServiceConfigurationPath)
+
+		_, statServiceConfigurationErr := os.Stat(linuxServiceConfigurationPath)
+		if statSystemdErr == nil && statServiceConfigurationErr == nil && runtime.GOOS == "linux" {
+			return extractLogsFromJournalCtl(ctx)
+		}
+
 		path, err := getServiceLogPath()
 		if err != nil {
 			return nil, err

diagnostic/log_collector_utils.go

@@ -12,7 +12,16 @@ func PipeCommandOutputToFile(command *exec.Cmd, outputHandle *os.File) (*LogInfo
 	stdoutReader, err := command.StdoutPipe()
 	if err != nil {
 		return nil, fmt.Errorf(
-			"error retrieving output from command '%s': %w",
+			"error retrieving stdout from command '%s': %w",
+			command.String(),
+			err,
+		)
+	}
+
+	stderrReader, err := command.StderrPipe()
+	if err != nil {
+		return nil, fmt.Errorf(
+			"error retrieving stderr from command '%s': %w",
 			command.String(),
 			err,
 		)
@@ -29,7 +38,17 @@ func PipeCommandOutputToFile(command *exec.Cmd, outputHandle *os.File) (*LogInfo
 	_, err = io.Copy(outputHandle, stdoutReader)
 	if err != nil {
 		return nil, fmt.Errorf(
-			"error copying output from %s to file %s: %w",
+			"error copying stdout from %s to file %s: %w",
+			command.String(),
+			outputHandle.Name(),
+			err,
+		)
+	}
+
+	_, err = io.Copy(outputHandle, stderrReader)
+	if err != nil {
+		return nil, fmt.Errorf(
+			"error copying stderr from %s to file %s: %w",
 			command.String(),
 			outputHandle.Name(),
 			err,

diagnostic/system_collector.go

@@ -1,6 +1,82 @@
 package diagnostic
 
-import "context"
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"strings"
+)
+
+type SystemInformationError struct {
+	Err     error  `json:"error"`
+	RawInfo string `json:"rawInfo"`
+}
+
+func (err SystemInformationError) Error() string {
+	return err.Err.Error()
+}
+
+func (err SystemInformationError) MarshalJSON() ([]byte, error) {
+	s := map[string]string{
+		"error":   err.Err.Error(),
+		"rawInfo": err.RawInfo,
+	}
+
+	return json.Marshal(s)
+}
+
+type SystemInformationGeneralError struct {
+	OperatingSystemInformationError error
+	MemoryInformationError          error
+	FileDescriptorsInformationError error
+	DiskVolumeInformationError      error
+}
+
+func (err SystemInformationGeneralError) Error() string {
+	builder := &strings.Builder{}
+	builder.WriteString("errors found:")
+
+	if err.OperatingSystemInformationError != nil {
+		builder.WriteString(err.OperatingSystemInformationError.Error() + ", ")
+	}
+
+	if err.MemoryInformationError != nil {
+		builder.WriteString(err.MemoryInformationError.Error() + ", ")
+	}
+
+	if err.FileDescriptorsInformationError != nil {
+		builder.WriteString(err.FileDescriptorsInformationError.Error() + ", ")
+	}
+
+	if err.DiskVolumeInformationError != nil {
+		builder.WriteString(err.DiskVolumeInformationError.Error() + ", ")
+	}
+
+	return builder.String()
+}
+
+func (err SystemInformationGeneralError) MarshalJSON() ([]byte, error) {
+	data := map[string]SystemInformationError{}
+
+	var sysErr SystemInformationError
+	if errors.As(err.OperatingSystemInformationError, &sysErr) {
+		data["operatingSystemInformationError"] = sysErr
+	}
+
+	if errors.As(err.MemoryInformationError, &sysErr) {
+		data["memoryInformationError"] = sysErr
+	}
+
+	if errors.As(err.FileDescriptorsInformationError, &sysErr) {
+		data["fileDescriptorsInformationError"] = sysErr
+	}
+
+	if errors.As(err.DiskVolumeInformationError, &sysErr) {
+		data["diskVolumeInformationError"] = sysErr
+	}
+
+	return json.Marshal(data)
+}
 
 type DiskVolumeInformation struct {
 	Name        string `json:"name"`        // represents the filesystem in linux/macos or device name in windows
@@ -17,17 +93,19 @@ func NewDiskVolumeInformation(name string, maximum, current uint64) *DiskVolumeI
 }
 
 type SystemInformation struct {
-	MemoryMaximum         uint64                   `json:"memoryMaximum"`         // represents the maximum memory of the system in kilobytes
-	MemoryCurrent         uint64                   `json:"memoryCurrent"`         // represents the system's memory in use in kilobytes
-	FileDescriptorMaximum uint64                   `json:"fileDescriptorMaximum"` // represents the maximum number of file descriptors of the system
-	FileDescriptorCurrent uint64                   `json:"fileDescriptorCurrent"` // represents the system's file descriptors in use
-	OsSystem              string                   `json:"osSystem"`              // represents the operating system name i.e.: linux, windows, darwin
-	HostName              string                   `json:"hostName"`              // represents the system host name
-	OsVersion             string                   `json:"osVersion"`             // detailed information about the system's release version level
-	OsRelease             string                   `json:"osRelease"`             // detailed information about the system's release
-	Architecture          string                   `json:"architecture"`          // represents the system's hardware platform i.e: arm64/amd64
-	CloudflaredVersion    string                   `json:"cloudflaredVersion"`    // the runtime version of cloudflared
-	Disk                  []*DiskVolumeInformation `json:"disk"`
+	MemoryMaximum         uint64                   `json:"memoryMaximum,omitempty"`         // represents the maximum memory of the system in kilobytes
+	MemoryCurrent         uint64                   `json:"memoryCurrent,omitempty"`         // represents the system's memory in use in kilobytes
+	FileDescriptorMaximum uint64                   `json:"fileDescriptorMaximum,omitempty"` // represents the maximum number of file descriptors of the system
+	FileDescriptorCurrent uint64                   `json:"fileDescriptorCurrent,omitempty"` // represents the system's file descriptors in use
+	OsSystem              string                   `json:"osSystem,omitempty"`              // represents the operating system name i.e.: linux, windows, darwin
+	HostName              string                   `json:"hostName,omitempty"`              // represents the system host name
+	OsVersion             string                   `json:"osVersion,omitempty"`             // detailed information about the system's release version level
+	OsRelease             string                   `json:"osRelease,omitempty"`             // detailed information about the system's release
+	Architecture          string                   `json:"architecture,omitempty"`          // represents the system's hardware platform i.e: arm64/amd64
+	CloudflaredVersion    string                   `json:"cloudflaredVersion,omitempty"`    // the runtime version of cloudflared
+	GoVersion             string                   `json:"goVersion,omitempty"`
+	GoArch                string                   `json:"goArch,omitempty"`
+	Disk                  []*DiskVolumeInformation `json:"disk,omitempty"`
 }
 
 func NewSystemInformation(
@@ -40,7 +118,9 @@ func NewSystemInformation(
 	osVersion,
 	osRelease,
 	architecture,
-	cloudflaredVersion string,
+	cloudflaredVersion,
+	goVersion,
+	goArchitecture string,
 	disk []*DiskVolumeInformation,
 ) *SystemInformation {
 	return &SystemInformation{
@@ -54,17 +134,17 @@ func NewSystemInformation(
 		osRelease,
 		architecture,
 		cloudflaredVersion,
+		goVersion,
+		goArchitecture,
 		disk,
 	}
 }
 
 type SystemCollector interface {
 	// If the collection is successful it will return `SystemInformation` struct,
-	// an empty string, and a nil error.
-	// In case there is an error a string with the raw data will be returned
-	// however the returned string not contain all the data points.
+	// and a nil error.
 	//
 	// This function expects that the caller sets the context timeout to prevent
 	// long-lived collectors.
-	Collect(ctx context.Context) (*SystemInformation, string, error)
+	Collect(ctx context.Context) (*SystemInformation, error)
 }

diagnostic/system_collector_linux.go

@@ -6,6 +6,7 @@ import (
 	"context"
 	"fmt"
 	"os/exec"
+	"runtime"
 	"strconv"
 	"strings"
 )
@@ -22,45 +23,74 @@ func NewSystemCollectorImpl(
 	}
 }
 
-func (collector *SystemCollectorImpl) Collect(ctx context.Context) (*SystemInformation, string, error) {
+func (collector *SystemCollectorImpl) Collect(ctx context.Context) (*SystemInformation, error) {
 	memoryInfo, memoryInfoRaw, memoryInfoErr := collectMemoryInformation(ctx)
 	fdInfo, fdInfoRaw, fdInfoErr := collectFileDescriptorInformation(ctx)
 	disks, disksRaw, diskErr := collectDiskVolumeInformationUnix(ctx)
 	osInfo, osInfoRaw, osInfoErr := collectOSInformationUnix(ctx)
 
+	var memoryMaximum, memoryCurrent, fileDescriptorMaximum, fileDescriptorCurrent uint64
+	var osSystem, name, osVersion, osRelease, architecture string
+	gerror := SystemInformationGeneralError{}
+
 	if memoryInfoErr != nil {
-		raw := RawSystemInformation(osInfoRaw, memoryInfoRaw, fdInfoRaw, disksRaw)
-		return nil, raw, memoryInfoErr
+		gerror.MemoryInformationError = SystemInformationError{
+			Err:     memoryInfoErr,
+			RawInfo: memoryInfoRaw,
+		}
+	} else {
+		memoryMaximum = memoryInfo.MemoryMaximum
+		memoryCurrent = memoryInfo.MemoryCurrent
 	}
 
 	if fdInfoErr != nil {
-		raw := RawSystemInformation(osInfoRaw, memoryInfoRaw, fdInfoRaw, disksRaw)
-		return nil, raw, fdInfoErr
+		gerror.FileDescriptorsInformationError = SystemInformationError{
+			Err:     fdInfoErr,
+			RawInfo: fdInfoRaw,
+		}
+	} else {
+		fileDescriptorMaximum = fdInfo.FileDescriptorMaximum
+		fileDescriptorCurrent = fdInfo.FileDescriptorCurrent
 	}
 
 	if diskErr != nil {
-		raw := RawSystemInformation(osInfoRaw, memoryInfoRaw, fdInfoRaw, disksRaw)
-		return nil, raw, diskErr
+		gerror.DiskVolumeInformationError = SystemInformationError{
+			Err:     diskErr,
+			RawInfo: disksRaw,
+		}
 	}
 
 	if osInfoErr != nil {
-		raw := RawSystemInformation(osInfoRaw, memoryInfoRaw, fdInfoRaw, disksRaw)
-		return nil, raw, osInfoErr
+		gerror.OperatingSystemInformationError = SystemInformationError{
+			Err:     osInfoErr,
+			RawInfo: osInfoRaw,
+		}
+	} else {
+		osSystem = osInfo.OsSystem
+		name = osInfo.Name
+		osVersion = osInfo.OsVersion
+		osRelease = osInfo.OsRelease
+		architecture = osInfo.Architecture
 	}
 
-	return NewSystemInformation(
-		memoryInfo.MemoryMaximum,
-		memoryInfo.MemoryCurrent,
-		fdInfo.FileDescriptorMaximum,
-		fdInfo.FileDescriptorCurrent,
-		osInfo.OsSystem,
-		osInfo.Name,
-		osInfo.OsVersion,
-		osInfo.OsRelease,
-		osInfo.Architecture,
-		collector.version,
+	cloudflaredVersion := collector.version
+	info := NewSystemInformation(
+		memoryMaximum,
+		memoryCurrent,
+		fileDescriptorMaximum,
+		fileDescriptorCurrent,
+		osSystem,
+		name,
+		osVersion,
+		osRelease,
+		architecture,
+		cloudflaredVersion,
+		runtime.Version(),
+		runtime.GOARCH,
 		disks,
-	), "", nil
+	)
+
+	return info, gerror
 }
 
 func collectMemoryInformation(ctx context.Context) (*MemoryInformation, string, error) {

diagnostic/system_collector_macos.go

@@ -6,6 +6,7 @@ import (
 	"context"
 	"fmt"
 	"os/exec"
+	"runtime"
 	"strconv"
 )
 
@@ -21,41 +22,80 @@ func NewSystemCollectorImpl(
 	}
 }
 
-func (collector *SystemCollectorImpl) Collect(ctx context.Context) (*SystemInformation, string, error) {
+func (collector *SystemCollectorImpl) Collect(ctx context.Context) (*SystemInformation, error) {
 	memoryInfo, memoryInfoRaw, memoryInfoErr := collectMemoryInformation(ctx)
 	fdInfo, fdInfoRaw, fdInfoErr := collectFileDescriptorInformation(ctx)
 	disks, disksRaw, diskErr := collectDiskVolumeInformationUnix(ctx)
 	osInfo, osInfoRaw, osInfoErr := collectOSInformationUnix(ctx)
 
+	var memoryMaximum, memoryCurrent, fileDescriptorMaximum, fileDescriptorCurrent uint64
+	var osSystem, name, osVersion, osRelease, architecture string
+
+	err := SystemInformationGeneralError{
+		OperatingSystemInformationError: nil,
+		MemoryInformationError:          nil,
+		FileDescriptorsInformationError: nil,
+		DiskVolumeInformationError:      nil,
+	}
+
 	if memoryInfoErr != nil {
-		return nil, RawSystemInformation(osInfoRaw, memoryInfoRaw, fdInfoRaw, disksRaw), memoryInfoErr
+		err.MemoryInformationError = SystemInformationError{
+			Err:     memoryInfoErr,
+			RawInfo: memoryInfoRaw,
+		}
+	} else {
+		memoryMaximum = memoryInfo.MemoryMaximum
+		memoryCurrent = memoryInfo.MemoryCurrent
 	}
 
 	if fdInfoErr != nil {
-		return nil, RawSystemInformation(osInfoRaw, memoryInfoRaw, fdInfoRaw, disksRaw), fdInfoErr
+		err.FileDescriptorsInformationError = SystemInformationError{
+			Err:     fdInfoErr,
+			RawInfo: fdInfoRaw,
+		}
+	} else {
+		fileDescriptorMaximum = fdInfo.FileDescriptorMaximum
+		fileDescriptorCurrent = fdInfo.FileDescriptorCurrent
 	}
 
 	if diskErr != nil {
-		return nil, RawSystemInformation(osInfoRaw, memoryInfoRaw, fdInfoRaw, disksRaw), diskErr
+		err.DiskVolumeInformationError = SystemInformationError{
+			Err:     diskErr,
+			RawInfo: disksRaw,
+		}
 	}
 
 	if osInfoErr != nil {
-		return nil, RawSystemInformation(osInfoRaw, memoryInfoRaw, fdInfoRaw, disksRaw), osInfoErr
+		err.OperatingSystemInformationError = SystemInformationError{
+			Err:     osInfoErr,
+			RawInfo: osInfoRaw,
+		}
+	} else {
+		osSystem = osInfo.OsSystem
+		name = osInfo.Name
+		osVersion = osInfo.OsVersion
+		osRelease = osInfo.OsRelease
+		architecture = osInfo.Architecture
 	}
 
-	return NewSystemInformation(
-		memoryInfo.MemoryMaximum,
-		memoryInfo.MemoryCurrent,
-		fdInfo.FileDescriptorMaximum,
-		fdInfo.FileDescriptorCurrent,
-		osInfo.OsSystem,
-		osInfo.Name,
-		osInfo.OsVersion,
-		osInfo.OsRelease,
-		osInfo.Architecture,
-		collector.version,
+	cloudflaredVersion := collector.version
+	info := NewSystemInformation(
+		memoryMaximum,
+		memoryCurrent,
+		fileDescriptorMaximum,
+		fileDescriptorCurrent,
+		osSystem,
+		name,
+		osVersion,
+		osRelease,
+		architecture,
+		cloudflaredVersion,
+		runtime.Version(),
+		runtime.GOARCH,
 		disks,
-	), "", nil
+	)
+
+	return info, err
 }
 
 func collectFileDescriptorInformation(ctx context.Context) (

diagnostic/system_collector_windows.go

@@ -6,6 +6,7 @@ import (
 	"context"
 	"fmt"
 	"os/exec"
+	"runtime"
 	"strconv"
 )
 
@@ -22,41 +23,70 @@ func NewSystemCollectorImpl(
 		version,
 	}
 }
-func (collector *SystemCollectorImpl) Collect(ctx context.Context) (*SystemInformation, string, error) {
+
+func (collector *SystemCollectorImpl) Collect(ctx context.Context) (*SystemInformation, error) {
 	memoryInfo, memoryInfoRaw, memoryInfoErr := collectMemoryInformation(ctx)
 	disks, disksRaw, diskErr := collectDiskVolumeInformation(ctx)
 	osInfo, osInfoRaw, osInfoErr := collectOSInformation(ctx)
 
+	var memoryMaximum, memoryCurrent, fileDescriptorMaximum, fileDescriptorCurrent uint64
+	var osSystem, name, osVersion, osRelease, architecture string
+
+	err := SystemInformationGeneralError{
+		OperatingSystemInformationError: nil,
+		MemoryInformationError:          nil,
+		FileDescriptorsInformationError: nil,
+		DiskVolumeInformationError:      nil,
+	}
+
 	if memoryInfoErr != nil {
-		raw := RawSystemInformation(osInfoRaw, memoryInfoRaw, "", disksRaw)
-		return nil, raw, memoryInfoErr
+		err.MemoryInformationError = SystemInformationError{
+			Err:     memoryInfoErr,
+			RawInfo: memoryInfoRaw,
+		}
+	} else {
+		memoryMaximum = memoryInfo.MemoryMaximum
+		memoryCurrent = memoryInfo.MemoryCurrent
 	}
 
 	if diskErr != nil {
-		raw := RawSystemInformation(osInfoRaw, memoryInfoRaw, "", disksRaw)
-		return nil, raw, diskErr
+		err.DiskVolumeInformationError = SystemInformationError{
+			Err:     diskErr,
+			RawInfo: disksRaw,
+		}
 	}
 
 	if osInfoErr != nil {
-		raw := RawSystemInformation(osInfoRaw, memoryInfoRaw, "", disksRaw)
-		return nil, raw, osInfoErr
+		err.OperatingSystemInformationError = SystemInformationError{
+			Err:     osInfoErr,
+			RawInfo: osInfoRaw,
+		}
+	} else {
+		osSystem = osInfo.OsSystem
+		name = osInfo.Name
+		osVersion = osInfo.OsVersion
+		osRelease = osInfo.OsRelease
+		architecture = osInfo.Architecture
 	}
 
-	return NewSystemInformation(
-		memoryInfo.MemoryMaximum,
-		memoryInfo.MemoryCurrent,
-		// For windows we leave both the fileDescriptorMaximum and fileDescriptorCurrent with zero
-		// since there is no obvious way to get this information.
-		0,
-		0,
-		osInfo.OsSystem,
-		osInfo.Name,
-		osInfo.OsVersion,
-		osInfo.OsRelease,
-		osInfo.Architecture,
-		collector.version,
+	cloudflaredVersion := collector.version
+	info := NewSystemInformation(
+		memoryMaximum,
+		memoryCurrent,
+		fileDescriptorMaximum,
+		fileDescriptorCurrent,
+		osSystem,
+		name,
+		osVersion,
+		osRelease,
+		architecture,
+		cloudflaredVersion,
+		runtime.Version(),
+		runtime.GOARCH,
 		disks,
-	), "", nil
+	)
+
+	return info, err
 }
 
 func collectMemoryInformation(ctx context.Context) (*MemoryInformation, string, error) {

metrics/metrics.go

@@ -29,7 +29,7 @@ var Runtime = "host"
 
 func GetMetricsDefaultAddress(runtimeType string) string {
 	// When issuing the diagnostic command we may have to reach a server that is
-	// running in a virtual enviroment and in that case we must bind to 0.0.0.0
+	// running in a virtual environment and in that case we must bind to 0.0.0.0
 	// otherwise the server won't be reachable.
 	switch runtimeType {
 	case "virtual":

quic/v3/muxer.go

@@ -65,12 +65,11 @@ type datagramConn struct {
 	icmpRouter     ingress.ICMPRouter
 	metrics        Metrics
 	logger         *zerolog.Logger
-
-	datagrams  chan []byte
-	readErrors chan error
+	datagrams      chan []byte
+	readErrors     chan error
 
 	icmpEncoderPool sync.Pool // a pool of *packet.Encoder
-	icmpDecoder     *packet.ICMPDecoder
+	icmpDecoderPool sync.Pool
 }
 
 func NewDatagramConn(conn QuicConnection, sessionManager SessionManager, icmpRouter ingress.ICMPRouter, index uint8, metrics Metrics, logger *zerolog.Logger) DatagramConn {
@@ -89,7 +88,11 @@ func NewDatagramConn(conn QuicConnection, sessionManager SessionManager, icmpRou
 				return packet.NewEncoder()
 			},
 		},
-		icmpDecoder: packet.NewICMPDecoder(),
+		icmpDecoderPool: sync.Pool{
+			New: func() any {
+				return packet.NewICMPDecoder()
+			},
+		},
 	}
 }
 
@@ -367,7 +370,16 @@ func (c *datagramConn) handleICMPPacket(datagram *ICMPDatagram) {
 
 	// Decode the provided ICMPDatagram as an ICMP packet
 	rawPacket := packet.RawPacket{Data: datagram.Payload}
-	icmp, err := c.icmpDecoder.Decode(rawPacket)
+	cachedDecoder := c.icmpDecoderPool.Get()
+	defer c.icmpDecoderPool.Put(cachedDecoder)
+	decoder, ok := cachedDecoder.(*packet.ICMPDecoder)
+	if !ok {
+		c.logger.Error().Msg("Could not get ICMPDecoder from the pool. Dropping packet")
+		return
+	}
+
+	icmp, err := decoder.Decode(rawPacket)
+
 	if err != nil {
 		c.logger.Err(err).Msgf("unable to marshal icmp packet")
 		return

quic/v3/muxer_test.go

@@ -4,13 +4,17 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"fmt"
 	"net"
 	"net/netip"
 	"slices"
+	"sort"
 	"sync"
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
+
 	"github.com/google/gopacket/layers"
 	"github.com/rs/zerolog"
 	"golang.org/x/net/icmp"
@@ -304,6 +308,91 @@ func TestDatagramConnServe(t *testing.T) {
 	assertContextClosed(t, ctx, done, cancel)
 }
 
+// This test exists because decoding multiple packets in parallel with the same decoder
+// instances causes inteference resulting in multiple different raw packets being decoded
+// as the same decoded packet.
+func TestDatagramConnServeDecodeMultipleICMPInParallel(t *testing.T) {
+	log := zerolog.Nop()
+	quic := newMockQuicConn()
+	session := newMockSession()
+	sessionManager := mockSessionManager{session: &session}
+	router := newMockICMPRouter()
+	conn := v3.NewDatagramConn(quic, &sessionManager, router, 0, &noopMetrics{}, &log)
+
+	// Setup the muxer
+	ctx, cancel := context.WithCancelCause(context.Background())
+	defer cancel(errors.New("other error"))
+	done := make(chan error, 1)
+	go func() {
+		done <- conn.Serve(ctx)
+	}()
+
+	packetCount := 100
+	packets := make([]*packet.ICMP, 100)
+	ipTemplate := "10.0.0.%d"
+	for i := 1; i <= packetCount; i++ {
+		packets[i-1] = &packet.ICMP{
+			IP: &packet.IP{
+				Src:      netip.MustParseAddr("192.168.1.1"),
+				Dst:      netip.MustParseAddr(fmt.Sprintf(ipTemplate, i)),
+				Protocol: layers.IPProtocolICMPv4,
+				TTL:      20,
+			},
+			Message: &icmp.Message{
+				Type: ipv4.ICMPTypeEcho,
+				Code: 0,
+				Body: &icmp.Echo{
+					ID:   25821,
+					Seq:  58129,
+					Data: []byte("test"),
+				},
+			},
+		}
+	}
+
+	wg := sync.WaitGroup{}
+	var receivedPackets []*packet.ICMP
+	go func() {
+		for ctx.Err() == nil {
+			select {
+			case icmpPacket := <-router.recv:
+				receivedPackets = append(receivedPackets, icmpPacket)
+				wg.Done()
+			}
+		}
+	}()
+
+	for _, p := range packets {
+		// We increment here but only decrement when receiving the packet
+		wg.Add(1)
+		go func() {
+			datagram := newICMPDatagram(p)
+			quic.send <- datagram
+		}()
+	}
+
+	wg.Wait()
+
+	// If there were duplicates then we won't have the same number of IPs
+	packetSet := make(map[netip.Addr]*packet.ICMP, 0)
+	for _, p := range receivedPackets {
+		packetSet[p.Dst] = p
+	}
+	assert.Equal(t, len(packetSet), len(packets))
+
+	// Sort the slice by last byte of IP address (the one we increment for each destination)
+	// and then check that we have one match for each packet sent
+	sort.Slice(receivedPackets, func(i, j int) bool {
+		return receivedPackets[i].Dst.As4()[3] < receivedPackets[j].Dst.As4()[3]
+	})
+	for i, p := range receivedPackets {
+		assert.Equal(t, p.Dst, packets[i].Dst)
+	}
+
+	// Cancel the muxer Serve context and make sure it closes with the expected error
+	assertContextClosed(t, ctx, done, cancel)
+}
+
 func TestDatagramConnServe_RegisterTwice(t *testing.T) {
 	log := zerolog.Nop()
 	quic := newMockQuicConn()

quic/v3/session.go

@@ -89,7 +89,10 @@ func NewSession(
 	log *zerolog.Logger,
 ) Session {
 	logger := log.With().Str(logFlowID, id.String()).Logger()
-	closeChan := make(chan error, 1)
+	// closeChan has two slots to allow for both writers (the closeFn and the Serve routine) to both be able to
+	// write to the channel without blocking since there is only ever one value read from the closeChan by the
+	// waitForCloseCondition.
+	closeChan := make(chan error, 2)
 	session := &session{
 		id:             id,
 		closeAfterIdle: closeAfterIdle,

quic/v3/session_test.go

@@ -3,13 +3,14 @@ package v3_test
 import (
 	"context"
 	"errors"
+	"io"
 	"net"
 	"net/netip"
 	"slices"
-	"sync/atomic"
 	"testing"
 	"time"
 
+	"github.com/fortytw2/leaktest"
 	"github.com/rs/zerolog"
 
 	v3 "github.com/cloudflare/cloudflared/quic/v3"
@@ -32,45 +33,64 @@ func TestSessionNew(t *testing.T) {
 
 func testSessionWrite(t *testing.T, payload []byte) {
 	log := zerolog.Nop()
-	origin := newTestOrigin(makePayload(1280))
-	session := v3.NewSession(testRequestID, 5*time.Second, &origin, testOriginAddr, testLocalAddr, &noopEyeball{}, &noopMetrics{}, &log)
+	origin, server := net.Pipe()
+	defer origin.Close()
+	defer server.Close()
+	// Start origin server read
+	serverRead := make(chan []byte, 1)
+	go func() {
+		read := make([]byte, 1500)
+		server.Read(read[:])
+		serverRead <- read
+	}()
+	// Create session and write to origin
+	session := v3.NewSession(testRequestID, 5*time.Second, origin, testOriginAddr, testLocalAddr, &noopEyeball{}, &noopMetrics{}, &log)
 	n, err := session.Write(payload)
+	defer session.Close()
 	if err != nil {
 		t.Fatal(err)
 	}
 	if n != len(payload) {
 		t.Fatal("unable to write the whole payload")
 	}
-	if !slices.Equal(payload, origin.write[:len(payload)]) {
+
+	read := <-serverRead
+	if !slices.Equal(payload, read[:len(payload)]) {
 		t.Fatal("payload provided from origin and read value are not the same")
 	}
 }
 
 func TestSessionWrite_Max(t *testing.T) {
+	defer leaktest.Check(t)()
 	payload := makePayload(1280)
 	testSessionWrite(t, payload)
 }
 
 func TestSessionWrite_Min(t *testing.T) {
+	defer leaktest.Check(t)()
 	payload := makePayload(0)
 	testSessionWrite(t, payload)
 }
 
 func TestSessionServe_OriginMax(t *testing.T) {
+	defer leaktest.Check(t)()
 	payload := makePayload(1280)
 	testSessionServe_Origin(t, payload)
 }
 
 func TestSessionServe_OriginMin(t *testing.T) {
+	defer leaktest.Check(t)()
 	payload := makePayload(0)
 	testSessionServe_Origin(t, payload)
 }
 
 func testSessionServe_Origin(t *testing.T, payload []byte) {
 	log := zerolog.Nop()
+	origin, server := net.Pipe()
+	defer origin.Close()
+	defer server.Close()
 	eyeball := newMockEyeball()
-	origin := newTestOrigin(payload)
-	session := v3.NewSession(testRequestID, 3*time.Second, &origin, testOriginAddr, testLocalAddr, &eyeball, &noopMetrics{}, &log)
+	session := v3.NewSession(testRequestID, 3*time.Second, origin, testOriginAddr, testLocalAddr, &eyeball, &noopMetrics{}, &log)
 	defer session.Close()
 
 	ctx, cancel := context.WithCancelCause(context.Background())
@@ -80,13 +100,19 @@ func testSessionServe_Origin(t *testing.T, payload []byte) {
 		done <- session.Serve(ctx)
 	}()
 
+	// Write from the origin server
+	_, err := server.Write(payload)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	select {
 	case data := <-eyeball.recvData:
 		// check received data matches provided from origin
 		expectedData := makePayload(1500)
 		v3.MarshalPayloadHeaderTo(testRequestID, expectedData[:])
 		copy(expectedData[17:], payload)
-		if !slices.Equal(expectedData[:17+len(payload)], data) {
+		if !slices.Equal(expectedData[:v3.DatagramPayloadHeaderLen+len(payload)], data) {
 			t.Fatal("expected datagram did not equal expected")
 		}
 		cancel(expectedContextCanceled)
@@ -95,7 +121,7 @@ func testSessionServe_Origin(t *testing.T, payload []byte) {
 		t.Fatal(err)
 	}
 
-	err := <-done
+	err = <-done
 	if !errors.Is(err, context.Canceled) {
 		t.Fatal(err)
 	}
@@ -105,11 +131,14 @@ func testSessionServe_Origin(t *testing.T, payload []byte) {
 }
 
 func TestSessionServe_OriginTooLarge(t *testing.T) {
+	defer leaktest.Check(t)()
 	log := zerolog.Nop()
 	eyeball := newMockEyeball()
 	payload := makePayload(1281)
-	origin := newTestOrigin(payload)
-	session := v3.NewSession(testRequestID, 2*time.Second, &origin, testOriginAddr, testLocalAddr, &eyeball, &noopMetrics{}, &log)
+	origin, server := net.Pipe()
+	defer origin.Close()
+	defer server.Close()
+	session := v3.NewSession(testRequestID, 2*time.Second, origin, testOriginAddr, testLocalAddr, &eyeball, &noopMetrics{}, &log)
 	defer session.Close()
 
 	done := make(chan error)
@@ -117,6 +146,12 @@ func TestSessionServe_OriginTooLarge(t *testing.T) {
 		done <- session.Serve(context.Background())
 	}()
 
+	// Attempt to write a payload too large from the origin
+	_, err := server.Write(payload)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	select {
 	case data := <-eyeball.recvData:
 		// we never expect a read to make it here because the origin provided a payload that is too large
@@ -130,6 +165,7 @@ func TestSessionServe_OriginTooLarge(t *testing.T) {
 }
 
 func TestSessionServe_Migrate(t *testing.T) {
+	defer leaktest.Check(t)()
 	log := zerolog.Nop()
 	eyeball := newMockEyeball()
 	pipe1, pipe2 := net.Pipe()
@@ -186,6 +222,7 @@ func TestSessionServe_Migrate(t *testing.T) {
 }
 
 func TestSessionServe_Migrate_CloseContext2(t *testing.T) {
+	defer leaktest.Check(t)()
 	log := zerolog.Nop()
 	eyeball := newMockEyeball()
 	pipe1, pipe2 := net.Pipe()
@@ -245,39 +282,48 @@ func TestSessionServe_Migrate_CloseContext2(t *testing.T) {
 }
 
 func TestSessionClose_Multiple(t *testing.T) {
+	defer leaktest.Check(t)()
 	log := zerolog.Nop()
-	origin := newTestOrigin(makePayload(128))
-	session := v3.NewSession(testRequestID, 5*time.Second, &origin, testOriginAddr, testLocalAddr, &noopEyeball{}, &noopMetrics{}, &log)
+	origin, server := net.Pipe()
+	defer origin.Close()
+	defer server.Close()
+	session := v3.NewSession(testRequestID, 5*time.Second, origin, testOriginAddr, testLocalAddr, &noopEyeball{}, &noopMetrics{}, &log)
 	err := session.Close()
 	if err != nil {
 		t.Fatal(err)
 	}
-	if !origin.closed.Load() {
-		t.Fatal("origin wasn't closed")
+	b := [1500]byte{}
+	_, err = server.Read(b[:])
+	if !errors.Is(err, io.EOF) {
+		t.Fatalf("origin server connection should be closed: %s", err)
 	}
-	// Reset the closed status to make sure it isn't closed again
-	origin.closed.Store(false)
 	// subsequent closes shouldn't call close again or cause any errors
 	err = session.Close()
 	if err != nil {
 		t.Fatal(err)
 	}
-	if origin.closed.Load() {
-		t.Fatal("origin was incorrectly closed twice")
+	_, err = server.Read(b[:])
+	if !errors.Is(err, io.EOF) {
+		t.Fatalf("origin server connection should still be closed: %s", err)
 	}
 }
 
 func TestSessionServe_IdleTimeout(t *testing.T) {
+	defer leaktest.Check(t)()
 	log := zerolog.Nop()
-	origin := newTestIdleOrigin(10 * time.Second) // Make idle time longer than closeAfterIdle
+	origin, server := net.Pipe()
+	defer origin.Close()
+	defer server.Close()
 	closeAfterIdle := 2 * time.Second
-	session := v3.NewSession(testRequestID, closeAfterIdle, &origin, testOriginAddr, testLocalAddr, &noopEyeball{}, &noopMetrics{}, &log)
+	session := v3.NewSession(testRequestID, closeAfterIdle, origin, testOriginAddr, testLocalAddr, &noopEyeball{}, &noopMetrics{}, &log)
 	err := session.Serve(context.Background())
 	if !errors.Is(err, v3.SessionIdleErr{}) {
 		t.Fatal(err)
 	}
 	// session should be closed
-	if !origin.closed {
+	b := [1500]byte{}
+	_, err = server.Read(b[:])
+	if !errors.Is(err, io.EOF) {
 		t.Fatalf("session should be closed after Serve returns")
 	}
 	// closing a session again should not return an error
@@ -288,12 +334,14 @@ func TestSessionServe_IdleTimeout(t *testing.T) {
 }
 
 func TestSessionServe_ParentContextCanceled(t *testing.T) {
+	defer leaktest.Check(t)()
 	log := zerolog.Nop()
-	// Make idle time and idle timeout longer than closeAfterIdle
-	origin := newTestIdleOrigin(10 * time.Second)
+	origin, server := net.Pipe()
+	defer origin.Close()
+	defer server.Close()
 	closeAfterIdle := 10 * time.Second
 
-	session := v3.NewSession(testRequestID, closeAfterIdle, &origin, testOriginAddr, testLocalAddr, &noopEyeball{}, &noopMetrics{}, &log)
+	session := v3.NewSession(testRequestID, closeAfterIdle, origin, testOriginAddr, testLocalAddr, &noopEyeball{}, &noopMetrics{}, &log)
 	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
 	defer cancel()
 	err := session.Serve(ctx)
@@ -301,7 +349,9 @@ func TestSessionServe_ParentContextCanceled(t *testing.T) {
 		t.Fatal(err)
 	}
 	// session should be closed
-	if !origin.closed {
+	b := [1500]byte{}
+	_, err = server.Read(b[:])
+	if !errors.Is(err, io.EOF) {
 		t.Fatalf("session should be closed after Serve returns")
 	}
 	// closing a session again should not return an error
@@ -312,6 +362,7 @@ func TestSessionServe_ParentContextCanceled(t *testing.T) {
 }
 
 func TestSessionServe_ReadErrors(t *testing.T) {
+	defer leaktest.Check(t)()
 	log := zerolog.Nop()
 	origin := newTestErrOrigin(net.ErrClosed, nil)
 	session := v3.NewSession(testRequestID, 30*time.Second, &origin, testOriginAddr, testLocalAddr, &noopEyeball{}, &noopMetrics{}, &log)
@@ -321,72 +372,6 @@ func TestSessionServe_ReadErrors(t *testing.T) {
 	}
 }
 
-type testOrigin struct {
-	// bytes from Write
-	write []byte
-	// bytes provided to Read
-	read     []byte
-	readOnce atomic.Bool
-	closed   atomic.Bool
-}
-
-func newTestOrigin(payload []byte) testOrigin {
-	return testOrigin{
-		read: payload,
-	}
-}
-
-func (o *testOrigin) Read(p []byte) (n int, err error) {
-	if o.closed.Load() {
-		return -1, net.ErrClosed
-	}
-	if o.readOnce.Load() {
-		// We only want to provide one read so all other reads will be blocked
-		time.Sleep(10 * time.Second)
-	}
-	o.readOnce.Store(true)
-	return copy(p, o.read), nil
-}
-
-func (o *testOrigin) Write(p []byte) (n int, err error) {
-	if o.closed.Load() {
-		return -1, net.ErrClosed
-	}
-	o.write = make([]byte, len(p))
-	copy(o.write, p)
-	return len(p), nil
-}
-
-func (o *testOrigin) Close() error {
-	o.closed.Store(true)
-	return nil
-}
-
-type testIdleOrigin struct {
-	duration time.Duration
-	closed   bool
-}
-
-func newTestIdleOrigin(d time.Duration) testIdleOrigin {
-	return testIdleOrigin{
-		duration: d,
-	}
-}
-
-func (o *testIdleOrigin) Read(p []byte) (n int, err error) {
-	time.Sleep(o.duration)
-	return -1, nil
-}
-
-func (o *testIdleOrigin) Write(p []byte) (n int, err error) {
-	return 0, nil
-}
-
-func (o *testIdleOrigin) Close() error {
-	o.closed = true
-	return nil
-}
-
 type testErrOrigin struct {
 	readErr  error
 	writeErr error

release_pkgs.py

@@ -113,7 +113,7 @@ class PkgCreator:
 
     def create_rpm_pkgs(self, artifacts_path, gpg_key_name):
         self._setup_rpm_pkg_directories(artifacts_path, gpg_key_name)
-        p = Popen(["createrepo", "./rpm"], stdout=PIPE, stderr=PIPE)
+        p = Popen(["createrepo_c", "./rpm"], stdout=PIPE, stderr=PIPE)
         out, err = p.communicate()
         if p.returncode != 0:
             print(f"create rpm_pkgs result => {out}, {err}")