From ab2fef18fcb61b5029483778b20cd0b84077d856 Mon Sep 17 00:00:00 2001 From: cartermp Date: Wed, 12 Jul 2023 09:40:07 -0700 Subject: [PATCH 1/8] Handle trailing slash on endpoint for otlphttp --- exporter/otlphttpexporter/factory.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/exporter/otlphttpexporter/factory.go b/exporter/otlphttpexporter/factory.go index 751738ba963..ac31794d77d 100644 --- a/exporter/otlphttpexporter/factory.go +++ b/exporter/otlphttpexporter/factory.go @@ -61,6 +61,9 @@ func composeSignalURL(oCfg *Config, signalOverrideURL string, signalName string) case oCfg.Endpoint == "": return "", fmt.Errorf("either endpoint or %s_endpoint must be specified", signalName) default: + if oCfg.Endpoint[len(oCfg.Endpoint)-1] == '/' { + return oCfg.Endpoint + "v1/" + signalName, nil + } return oCfg.Endpoint + "/v1/" + signalName, nil } } From cf3a20847ca402b2358e0ef182c8f94d13037dda Mon Sep 17 00:00:00 2001 From: cartermp Date: Wed, 12 Jul 2023 09:43:43 -0700 Subject: [PATCH 2/8] use strings --- exporter/otlphttpexporter/factory.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/exporter/otlphttpexporter/factory.go b/exporter/otlphttpexporter/factory.go index ac31794d77d..06dc8131897 100644 --- a/exporter/otlphttpexporter/factory.go +++ b/exporter/otlphttpexporter/factory.go @@ -8,6 +8,7 @@ import ( "fmt" "net/url" "time" + "strings" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configcompression" @@ -61,7 +62,7 @@ func composeSignalURL(oCfg *Config, signalOverrideURL string, signalName string) case oCfg.Endpoint == "": return "", fmt.Errorf("either endpoint or %s_endpoint must be specified", signalName) default: - if oCfg.Endpoint[len(oCfg.Endpoint)-1] == '/' { + if strings.HasSuffix(oCfg.Endpoint, "/") { return oCfg.Endpoint + "v1/" + signalName, nil } return oCfg.Endpoint + "/v1/" + signalName, nil From 3d56e6402149a49cfeb06a806c1d75891441ba5a Mon Sep 17 00:00:00 2001 From: cartermp Date: Wed, 12 Jul 2023 10:04:11 -0700 Subject: [PATCH 3/8] appease the lint robot --- exporter/otlphttpexporter/factory.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporter/otlphttpexporter/factory.go b/exporter/otlphttpexporter/factory.go index 06dc8131897..4bdbcdf40fd 100644 --- a/exporter/otlphttpexporter/factory.go +++ b/exporter/otlphttpexporter/factory.go @@ -7,8 +7,8 @@ import ( "context" "fmt" "net/url" - "time" "strings" + "time" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configcompression" From 8c25c8bf6b495645274ea2e628e22c702b2276da Mon Sep 17 00:00:00 2001 From: cartermp Date: Wed, 12 Jul 2023 10:16:44 -0700 Subject: [PATCH 4/8] add changelog entry --- .chloggen/otlphttpexporter-trailing-slash.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100755 .chloggen/otlphttpexporter-trailing-slash.yaml diff --git a/.chloggen/otlphttpexporter-trailing-slash.yaml b/.chloggen/otlphttpexporter-trailing-slash.yaml new file mode 100755 index 00000000000..55d16280f2d --- /dev/null +++ b/.chloggen/otlphttpexporter-trailing-slash.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: otlphttpexporter + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Add support for trailing slash in endpoint URL + +# One or more tracking issues or pull requests related to the change +issues: [] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: From 085ee756bea12f75b6ddb5dc8f36aceed75bd47f Mon Sep 17 00:00:00 2001 From: cartermp Date: Wed, 12 Jul 2023 10:31:19 -0700 Subject: [PATCH 5/8] Add test --- exporter/otlphttpexporter/factory_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/exporter/otlphttpexporter/factory_test.go b/exporter/otlphttpexporter/factory_test.go index 286f45cc923..cb0aef75557 100644 --- a/exporter/otlphttpexporter/factory_test.go +++ b/exporter/otlphttpexporter/factory_test.go @@ -193,3 +193,20 @@ func TestCreateLogsExporter(t *testing.T) { require.Nil(t, err) require.NotNil(t, oexp) } + +func TestComposeSignalURL(t *testing.T) { + factory := NewFactory() + cfg := factory.CreateDefaultConfig().(*Config) + + // Has slash at end + cfg.HTTPClientSettings.Endpoint = "http://www.gooddeals.com/" + url, err := composeSignalURL(cfg, "", "traces") + require.NoError(t, err) + assert.Equal(t, "http://www.gooddeals.com/v1/traces", url) + + // No slash at end + cfg.HTTPClientSettings.Endpoint = "http://www.gooddeals.com" + url, err = composeSignalURL(cfg, "", "traces") + require.NoError(t, err) + assert.Equal(t, "http://www.gooddeals.com/v1/traces", url) +} From 16c966452c8c2a6c6d7fb9c6a43dddd0732e8a76 Mon Sep 17 00:00:00 2001 From: cartermp Date: Wed, 12 Jul 2023 10:37:03 -0700 Subject: [PATCH 6/8] appease the robots --- .chloggen/otlphttpexporter-trailing-slash.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.chloggen/otlphttpexporter-trailing-slash.yaml b/.chloggen/otlphttpexporter-trailing-slash.yaml index 55d16280f2d..84859c2b78f 100755 --- a/.chloggen/otlphttpexporter-trailing-slash.yaml +++ b/.chloggen/otlphttpexporter-trailing-slash.yaml @@ -5,10 +5,10 @@ change_type: enhancement component: otlphttpexporter # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). -note: Add support for trailing slash in endpoint URL +note: "Add support for trailing slash in endpoint URL" # One or more tracking issues or pull requests related to the change -issues: [] +issues: [8084] # (Optional) One or more lines of additional information to render under the primary note. # These lines will be padded with 2 spaces and then inserted directly into the document. From 20cc277d9e3b910fcf8147e8afeae8ed766e1cd1 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Wed, 12 Jul 2023 13:49:35 -0700 Subject: [PATCH 7/8] Apply suggestions from code review Co-authored-by: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> --- .chloggen/otlphttpexporter-trailing-slash.yaml | 2 +- exporter/otlphttpexporter/factory_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.chloggen/otlphttpexporter-trailing-slash.yaml b/.chloggen/otlphttpexporter-trailing-slash.yaml index 84859c2b78f..ae45d0d4289 100755 --- a/.chloggen/otlphttpexporter-trailing-slash.yaml +++ b/.chloggen/otlphttpexporter-trailing-slash.yaml @@ -5,7 +5,7 @@ change_type: enhancement component: otlphttpexporter # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). -note: "Add support for trailing slash in endpoint URL" +note: Add support for trailing slash in endpoint URL # One or more tracking issues or pull requests related to the change issues: [8084] diff --git a/exporter/otlphttpexporter/factory_test.go b/exporter/otlphttpexporter/factory_test.go index cb0aef75557..03762abf89c 100644 --- a/exporter/otlphttpexporter/factory_test.go +++ b/exporter/otlphttpexporter/factory_test.go @@ -199,14 +199,14 @@ func TestComposeSignalURL(t *testing.T) { cfg := factory.CreateDefaultConfig().(*Config) // Has slash at end - cfg.HTTPClientSettings.Endpoint = "http://www.gooddeals.com/" + cfg.HTTPClientSettings.Endpoint = "http://localhost:4318/" url, err := composeSignalURL(cfg, "", "traces") require.NoError(t, err) - assert.Equal(t, "http://www.gooddeals.com/v1/traces", url) + assert.Equal(t, "http://localhost:4318/v1/traces", url) // No slash at end - cfg.HTTPClientSettings.Endpoint = "http://www.gooddeals.com" + cfg.HTTPClientSettings.Endpoint = "http://localhost:4318" url, err = composeSignalURL(cfg, "", "traces") require.NoError(t, err) - assert.Equal(t, "http://www.gooddeals.com/v1/traces", url) + assert.Equal(t, "http://localhost:4318/v1/traces", url) } From 11ca87cbb5024d6ce7543a4729acfb9bf28f051a Mon Sep 17 00:00:00 2001 From: Phillip Carter Date: Wed, 12 Jul 2023 14:11:49 -0700 Subject: [PATCH 8/8] Update .chloggen/otlphttpexporter-trailing-slash.yaml Co-authored-by: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> --- .chloggen/otlphttpexporter-trailing-slash.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/otlphttpexporter-trailing-slash.yaml b/.chloggen/otlphttpexporter-trailing-slash.yaml index ae45d0d4289..f146140d8a2 100755 --- a/.chloggen/otlphttpexporter-trailing-slash.yaml +++ b/.chloggen/otlphttpexporter-trailing-slash.yaml @@ -13,4 +13,4 @@ issues: [8084] # (Optional) One or more lines of additional information to render under the primary note. # These lines will be padded with 2 spaces and then inserted directly into the document. # Use pipe (|) for multiline entries. -subtext: +subtext: URLs like `http://localhost:4318/` will now be treated as if they were `http://localhost:4318`