Skip to content

Commit

Permalink
Remove URL-rewriting hack
Browse files Browse the repository at this point in the history
This commit removes a hack that rewrites the collector URL for use with
OpAmp. The collector now supports redirection, and we expect the Sumo
Logic backend to correctly redirect the client.
  • Loading branch information
echlebek committed Apr 17, 2024
1 parent 732df95 commit 243221c
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 103 deletions.
38 changes: 1 addition & 37 deletions pkg/extension/opampextension/opamp_agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,10 @@ import (
"context"
"fmt"
"net/http"
"net/url"
"os"
"path/filepath"
"reflect"
"runtime"
"strings"

"github.com/google/uuid"
"github.com/knadh/koanf/parsers/yaml"
Expand Down Expand Up @@ -100,14 +98,7 @@ func (o *opampAgent) Start(ctx context.Context, host component.Host) error {
return err
}

var baseURL string
if o.authExtension != nil {
baseURL = o.authExtension.BaseUrl()
}

if err := o.setEndpoint(baseURL); err != nil {
return err
}
o.endpoint = o.cfg.Endpoint

if o.authExtension == nil {
return o.startClient(ctx)
Expand Down Expand Up @@ -249,33 +240,6 @@ func (o *opampAgent) watchCredentials(ctx context.Context, callback func(ctx con
return nil
}

// setEndpoint sets the OpAMP endpoint based on the collector endpoint.
// This is a hack, and it should be removed when the backend is able to
// correctly redirect our OpAMP client to the correct URL.
func (o *opampAgent) setEndpoint(baseURL string) error {
if baseURL == "" {
o.endpoint = o.cfg.Endpoint
return nil
}
u, err := url.Parse(baseURL)
if err != nil {
return fmt.Errorf("url error, cannot set opamp endpoint: %s", err)
}

u.Scheme = "wss"
u.Path = "/v1/opamp"

// These replacements are specific to Sumo Logic's current domain naming,
// and are made provisionally for the OTRM beta. In the future, the backend
// will inform the agent of the correct OpAMP URL to use.
u.Host = strings.Replace(u.Host, "open-events", "opamp-events", 1)
u.Host = strings.Replace(u.Host, "open-collectors", "opamp-collectors", 1)

o.endpoint = u.String()

return nil
}

func newOpampAgent(cfg *Config, logger *zap.Logger, build component.BuildInfo, res pcommon.Resource) (*opampAgent, error) {
agentType := build.Command

Expand Down
66 changes: 0 additions & 66 deletions pkg/extension/opampextension/opamp_agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/extension/extensiontest"
semconv "go.opentelemetry.io/collector/semconv/v1.18.0"
)
Expand Down Expand Up @@ -486,68 +485,3 @@ func TestReload(t *testing.T) {
assert.NoError(t, o.Start(ctx, componenttest.NewNopHost()))
assert.NoError(t, o.Reload(ctx))
}

// To be removed when the OpAMP backend correctly advertises its URL.
// This test is badly written, it reaches into unexported fields to test
// their contents, but given that it will be removed in a few months, that
// shouldn't matter too much from a big picture perspective.
func TestHackSetEndpoint(t *testing.T) {
tests := []struct {
name string
url string
wantEndpoint string
}{
{
name: "empty url defaults to config endpoint",
wantEndpoint: "wss://example.com",
},
{
name: "url variant a",
url: "https://sumo-open-events.example.com",
wantEndpoint: "wss://sumo-opamp-events.example.com/v1/opamp",
},
{
name: "url variant b",
url: "https://sumo-open-collectors.example.com",
wantEndpoint: "wss://sumo-opamp-collectors.example.com/v1/opamp",
},
{
name: "url variant c",
url: "https://example.com",
wantEndpoint: "wss://example.com/v1/opamp",
},
{
name: "dev sumologic url",
url: "https://long-open-events.sumologic.net/api/v1",
wantEndpoint: "wss://long-opamp-events.sumologic.net/v1/opamp",
},
{
name: "prod sumologic url",
url: "https://open-collectors.sumologic.com/api/v1",
wantEndpoint: "wss://opamp-collectors.sumologic.com/v1/opamp",
},
{
name: "prod sumologic url with region",
url: "https://open-collectors.au.sumologic.com/api/v1/",
wantEndpoint: "wss://opamp-collectors.au.sumologic.com/v1/opamp",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
agent := &opampAgent{cfg: &Config{
ClientConfig: confighttp.ClientConfig{
Endpoint: "wss://example.com",
},
}}
if err := agent.setEndpoint(test.url); err != nil {
// can only happen with an invalid URL, which is quite hard
// to even come up with for Go's URL package
t.Fatal(err)
}
if got, want := agent.endpoint, test.wantEndpoint; got != want {
t.Errorf("didn't get expected endpoint: got %q, want %q", got, want)
}
})
}
}

0 comments on commit 243221c

Please sign in to comment.