From 09196522a7d6f56bc1639a5286ec2883f6e5b78b Mon Sep 17 00:00:00 2001 From: Chester Cheung Date: Wed, 2 Mar 2022 23:13:43 +0800 Subject: [PATCH] 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 * Update exporters/otlp/internal/config.go Co-authored-by: Tyler Yahn * Update exporters/otlp/internal/config.go Co-authored-by: Tyler Yahn * Update CHANGELOG.md Co-authored-by: Tyler Yahn * Update exporters/otlp/internal/config_test.go Co-authored-by: Tyler Yahn * Update exporters/otlp/internal/config_test.go Co-authored-by: Tyler Yahn * format the config_test.go * Update exporters/otlp/internal/config_test.go Co-authored-by: Sam Xie * Update exporters/otlp/internal/config_test.go Co-authored-by: Sam Xie * Update exporters/otlp/internal/config_test.go Co-authored-by: Sam Xie * Update exporters/otlp/internal/config_test.go Co-authored-by: Sam Xie * Update exporters/otlp/internal/config.go Co-authored-by: Sam Xie * Update exporters/otlp/internal/config.go Co-authored-by: Sam Xie * change URLPath to urlPath Co-authored-by: Tyler Yahn Co-authored-by: Sam Xie --- CHANGELOG.md | 1 + exporters/otlp/internal/config.go | 34 ++++++++ exporters/otlp/internal/config_test.go | 83 +++++++++++++++++++ .../otlpmetric/internal/otlpconfig/options.go | 15 +--- .../otlptrace/internal/otlpconfig/options.go | 15 +--- 5 files changed, 122 insertions(+), 26 deletions(-) create mode 100644 exporters/otlp/internal/config.go create mode 100644 exporters/otlp/internal/config_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 99f62c59cb6..73e1f723a65 100644 --- 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 new file mode 100644 index 00000000000..b3fd45d9d31 --- /dev/null +++ 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 new file mode 100644 index 00000000000..92a819a1f2b --- /dev/null +++ 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 index c2f442f998f..f2c8ee5d21a 100644 --- 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 index f874c146e25..56e83b85334 100644 --- 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 }