Unify OTLP path parsing/default Logic (#2639)

* unify otlp path parsing/default logic * add changelog * add license and unit test * remove else branch * increase unitt test coverage * add vanity import * Update exporters/otlp/internal/config.go Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> * Update exporters/otlp/internal/config.go Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> * Update exporters/otlp/internal/config.go Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> * Update CHANGELOG.md Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> * Update exporters/otlp/internal/config_test.go Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> * Update exporters/otlp/internal/config_test.go Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> * format the config_test.go * Update exporters/otlp/internal/config_test.go Co-authored-by: Sam Xie <sam@samxie.me> * Update exporters/otlp/internal/config_test.go Co-authored-by: Sam Xie <sam@samxie.me> * Update exporters/otlp/internal/config_test.go Co-authored-by: Sam Xie <sam@samxie.me> * Update exporters/otlp/internal/config_test.go Co-authored-by: Sam Xie <sam@samxie.me> * Update exporters/otlp/internal/config.go Co-authored-by: Sam Xie <sam@samxie.me> * Update exporters/otlp/internal/config.go Co-authored-by: Sam Xie <sam@samxie.me> * change URLPath to urlPath Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> Co-authored-by: Sam Xie <sam@samxie.me>
open-telemetry · Mar 2, 2022 · b66c902 · b66c902
1 parent 79f6bc1
commit b66c902
Show file tree

Hide file tree

Showing 5 changed files with 122 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,6 +24,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 - For tracestate's members, prepend the new element and remove the oldest one, which is over capacity (#2592)
 - Add event and link drop counts to the exported data from the `oltptrace` exporter. (#2601)
+- Unify path cleaning functionally in the `otlpmetric` and `otlptrace` config. (#2639)
 - Change the debug message from the `sdk/trace.BatchSpanProcessor` to reflect the count is cumulative. (#2640)
 
 ### Fixed

diff --git a/exporters/otlp/internal/config.go b/exporters/otlp/internal/config.go
@@ -0,0 +1,34 @@
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package internal contains common functionality for all OTLP exporters.
+package internal // import "go.opentelemetry.io/otel/exporters/otlp/internal"
+
+import (
+	"fmt"
+	"path"
+	"strings"
+)
+
+// CleanPath returns a path with all spaces trimmed and all redundancies removed. If urlPath is empty or cleaning it results in an empty string, defaultPath is returned instead.
+func CleanPath(urlPath string, defaultPath string) string {
+	tmp := path.Clean(strings.TrimSpace(urlPath))
+	if tmp == "." {
+		return defaultPath
+	}
+	if !path.IsAbs(tmp) {
+		tmp = fmt.Sprintf("/%s", tmp)
+	}
+	return tmp
+}
diff --git a/exporters/otlp/internal/config_test.go b/exporters/otlp/internal/config_test.go
@@ -0,0 +1,83 @@
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package internal
+
+import "testing"
+
+func TestCleanPath(t *testing.T) {
+	type args struct {
+		urlPath     string
+		defaultPath string
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{
+			name: "clean empty path",
+			args: args{
+				urlPath:     "",
+				defaultPath: "DefaultPath",
+			},
+			want: "DefaultPath",
+		},
+		{
+			name: "clean metrics path",
+			args: args{
+				urlPath:     "/prefix/v1/metrics",
+				defaultPath: "DefaultMetricsPath",
+			},
+			want: "/prefix/v1/metrics",
+		},
+		{
+			name: "clean traces path",
+			args: args{
+				urlPath:     "https://env_endpoint",
+				defaultPath: "DefaultTracesPath",
+			},
+			want: "/https:/env_endpoint",
+		},
+		{
+			name: "spaces trimmed",
+			args: args{
+				urlPath: " /dir",
+			},
+			want: "/dir",
+		},
+		{
+			name: "clean path empty",
+			args: args{
+				urlPath:     "dir/..",
+				defaultPath: "DefaultTracesPath",
+			},
+			want: "DefaultTracesPath",
+		},
+		{
+			name: "make absolute",
+			args: args{
+				urlPath: "dir/a",
+			},
+			want: "/dir/a",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := CleanPath(tt.args.urlPath, tt.args.defaultPath); got != tt.want {
+				t.Errorf("CleanPath() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
diff --git a/exporters/otlp/otlpmetric/internal/otlpconfig/options.go b/exporters/otlp/otlpmetric/internal/otlpconfig/options.go
@@ -17,8 +17,6 @@ package otlpconfig // import "go.opentelemetry.io/otel/exporters/otlp/otlpmetric
 import (
 	"crypto/tls"
 	"fmt"
-	"path"
-	"strings"
 	"time"
 
 	"google.golang.org/grpc"
@@ -27,6 +25,7 @@ import (
 	"google.golang.org/grpc/credentials/insecure"
 	"google.golang.org/grpc/encoding/gzip"
 
+	"go.opentelemetry.io/otel/exporters/otlp/internal"
 	"go.opentelemetry.io/otel/exporters/otlp/internal/retry"
 )
 
@@ -90,17 +89,7 @@ func NewHTTPConfig(opts ...HTTPOption) Config {
 	for _, opt := range opts {
 		cfg = opt.ApplyHTTPOption(cfg)
 	}
-
-	tmp := strings.TrimSpace(cfg.Metrics.URLPath)
-	if tmp == "" {
-		tmp = DefaultMetricsPath
-	} else {
-		tmp = path.Clean(tmp)
-		if !path.IsAbs(tmp) {
-			tmp = fmt.Sprintf("/%s", tmp)
-		}
-	}
-	cfg.Metrics.URLPath = tmp
+	cfg.Metrics.URLPath = internal.CleanPath(cfg.Metrics.URLPath, DefaultMetricsPath)
 	return cfg
 }
 

diff --git a/exporters/otlp/otlptrace/internal/otlpconfig/options.go b/exporters/otlp/otlptrace/internal/otlpconfig/options.go
@@ -17,8 +17,6 @@ package otlpconfig // import "go.opentelemetry.io/otel/exporters/otlp/otlptrace/
 import (
 	"crypto/tls"
 	"fmt"
-	"path"
-	"strings"
 	"time"
 
 	"google.golang.org/grpc"
@@ -27,6 +25,7 @@ import (
 	"google.golang.org/grpc/credentials/insecure"
 	"google.golang.org/grpc/encoding/gzip"
 
+	"go.opentelemetry.io/otel/exporters/otlp/internal"
 	"go.opentelemetry.io/otel/exporters/otlp/internal/retry"
 )
 
@@ -83,17 +82,7 @@ func NewHTTPConfig(opts ...HTTPOption) Config {
 	for _, opt := range opts {
 		cfg = opt.ApplyHTTPOption(cfg)
 	}
-
-	tmp := strings.TrimSpace(cfg.Traces.URLPath)
-	if tmp == "" {
-		tmp = DefaultTracesPath
-	} else {
-		tmp = path.Clean(tmp)
-		if !path.IsAbs(tmp) {
-			tmp = fmt.Sprintf("/%s", tmp)
-		}
-	}
-	cfg.Traces.URLPath = tmp
+	cfg.Traces.URLPath = internal.CleanPath(cfg.Traces.URLPath, DefaultTracesPath)
 	return cfg
 }