Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sumologicextension): use hostname as default collector name #918

Merged
merged 1 commit into from
Feb 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

This release introduces the following breaking changes:

- feat(sumologicextension): use hostname as default collector name [#918]

[#918]: https://github.com/SumoLogic/sumologic-otel-collector/pull/918
[Unreleased]: https://github.com/SumoLogic/sumologic-otel-collector/compare/v0.70.0-sumo-0...main

## [v0.70.0-sumo-0]

### Added
Expand Down
1 change: 0 additions & 1 deletion pkg/exporter/sumologicexporter/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ require (
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.2 // indirect
github.com/golang/snappy v0.0.4 // indirect
github.com/google/uuid v1.3.0 // indirect
github.com/hashicorp/errwrap v1.0.0 // indirect
github.com/hashicorp/go-multierror v1.1.1 // indirect
github.com/json-iterator/go v1.1.12 // indirect
Expand Down
6 changes: 3 additions & 3 deletions pkg/extension/sumologicextension/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ and can be used as an authenticator for the

- `install_token`: (required) collector install token for the Sumo Logic service, see
[help][credentials_help] for more details
- `collector_name`: name that will be used for registration; by default it is a
hostname followed by UUID
- `collector_name`: name that will be used for registration; by default the hostname is used. In the event of a conflict, a timestamp will be appended to the name. See [here][clobber] for more information.
- `collector_description`: collector description that will be used for registration
- `collector_category`: collector category that will be used for registration
- `collector_fields`: a map of key value pairs that will be used as collector
Expand All @@ -49,7 +48,7 @@ and can be used as an authenticator for the
- `collector_credentials_directory`: directory where state files with registration
info will be stored after successful collector registration
(default: `$HOME/.sumologic-otel-collector`)
- `clobber`: defines whether to delete any existing collector with the same name
- `clobber`: defines whether to delete any existing collector with the same name. See [here][clobber] for more information.
- `force_registration`: defines whether to force registration every time the
collector starts.
This will cause the collector to not look at the locally stored credentials
Expand All @@ -71,6 +70,7 @@ and can be used as an authenticator for the

[credentials_help]: https://help.sumologic.com/docs/manage/security/installation-tokens
[fields_help]: https://help.sumologic.com/docs/manage/fields
[clobber]: https://help.sumologic.com/docs/send-data/installed-collectors/collector-installation-reference/force-collectors-name-clobber/

## Example Config

Expand Down
7 changes: 5 additions & 2 deletions pkg/extension/sumologicextension/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
"time"

"github.com/cenkalti/backoff/v4"
"github.com/google/uuid"
ps "github.com/mitchellh/go-ps"
"github.com/shirou/gopsutil/v3/host"
"go.opentelemetry.io/collector/component"
Expand Down Expand Up @@ -130,7 +129,7 @@ func newSumologicExtension(conf *Config, logger *zap.Logger, id component.ID, bu
// and that we can reuse collector name save in credentials store.
if creds, err := credentialsStore.Get(hashKey); err != nil {
// If credentials file is not stored on filesystem generate collector name
collectorName = fmt.Sprintf("%s-%s", hostname, uuid.New())
collectorName = hostname
} else {
collectorName = creds.CollectorName
}
Expand Down Expand Up @@ -435,6 +434,10 @@ func (se *SumologicExtension) registerCollector(ctx context.Context, collectorNa
return credentials.CollectorCredentials{}, err
}

if collectorName != resp.CollectorName {
se.logger.Warn("Collector name already in use, registered modified name", zap.String("registered_name", resp.CollectorName))
}

Comment on lines +437 to +440
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, but not sure if we should mention clobber or any documentation here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I probably should add this behaviour to the documentation to begin with. I don't want to make this log message too long.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated documentation.

return credentials.CollectorCredentials{
CollectorName: collectorName,
Credentials: resp,
Expand Down
34 changes: 8 additions & 26 deletions pkg/extension/sumologicextension/extension_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,10 @@ import (
"crypto/sha256"
"encoding/base64"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"os"
"path"
"regexp"
"sync/atomic"
"testing"
"time"
Expand All @@ -41,10 +39,6 @@ import (
"github.com/SumoLogic/sumologic-otel-collector/pkg/extension/sumologicextension/credentials"
)

const (
uuidRegex = "[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}"
)

func TestMain(m *testing.M) {
// Enable the feature gates before all tests to avoid flaky tests.
err := featuregate.GlobalRegistry().Apply(map[string]bool{
Expand Down Expand Up @@ -588,10 +582,8 @@ func TestRegisterEmptyCollectorName(t *testing.T) {
se, err := newSumologicExtension(cfg, zap.NewNop(), component.NewID("sumologic"), "1.0.0")
require.NoError(t, err)
require.NoError(t, se.Start(context.Background(), componenttest.NewNopHost()))
regexPattern := fmt.Sprintf("%s-%s", hostname, uuidRegex)
matched, err := regexp.MatchString(regexPattern, se.collectorName)
require.NoError(t, err)
assert.True(t, matched)
require.Equal(t, hostname, se.collectorName)
}

func TestRegisterEmptyCollectorNameForceRegistration(t *testing.T) {
Expand Down Expand Up @@ -680,10 +672,7 @@ func TestRegisterEmptyCollectorNameForceRegistration(t *testing.T) {
require.NoError(t, se.Start(context.Background(), componenttest.NewNopHost()))
require.NoError(t, se.Shutdown(context.Background()))
assert.NotEmpty(t, se.collectorName)
regexPattern := fmt.Sprintf("%s-%s", hostname, uuidRegex)
matched, err := regexp.MatchString(regexPattern, se.collectorName)
require.NoError(t, err)
assert.True(t, matched)
assert.Equal(t, hostname, se.collectorName)
colCreds, err := se.credentialsStore.Get(se.hashKey)
require.NoError(t, err)
colName := colCreds.CollectorName
Expand Down Expand Up @@ -1005,10 +994,7 @@ func TestRegisterEmptyCollectorNameWithBackoff(t *testing.T) {
se, err := newSumologicExtension(cfg, zap.NewNop(), component.NewID("sumologic"), "1.0.0")
require.NoError(t, err)
require.NoError(t, se.Start(context.Background(), componenttest.NewNopHost()))
regexPattern := fmt.Sprintf("%s-%s", hostname, uuidRegex)
matched, err := regexp.MatchString(regexPattern, se.collectorName)
require.NoError(t, err)
assert.True(t, matched)
require.Equal(t, hostname, se.collectorName)
}

func TestRegisterEmptyCollectorNameUnrecoverableError(t *testing.T) {
Expand Down Expand Up @@ -1058,10 +1044,7 @@ func TestRegisterEmptyCollectorNameUnrecoverableError(t *testing.T) {
require.NoError(t, err)
require.EqualError(t, se.Start(context.Background(), componenttest.NewNopHost()),
"collector registration failed: failed to register the collector, got HTTP status code: 404")
regexPattern := fmt.Sprintf("%s-%s", hostname, uuidRegex)
matched, err := regexp.MatchString(regexPattern, se.collectorName)
require.NoError(t, err)
assert.True(t, matched)
require.Equal(t, hostname, se.collectorName)
}

func TestRegistrationRedirect(t *testing.T) {
Expand Down Expand Up @@ -1133,7 +1116,7 @@ func TestRegistrationRedirect(t *testing.T) {
// register
case 1:
require.Equal(t, registerUrl, req.URL.Path)
http.Redirect(w, req, destSrv.URL, 301)
http.Redirect(w, req, destSrv.URL, http.StatusMovedPermanently)

// should not produce any more requests
default:
Expand Down Expand Up @@ -1369,10 +1352,7 @@ func TestRegistrationRequestPayload(t *testing.T) {
se, err := newSumologicExtension(cfg, zap.NewNop(), component.NewID("sumologic"), "1.0.0")
require.NoError(t, err)
require.NoError(t, se.Start(context.Background(), componenttest.NewNopHost()))
regexPattern := fmt.Sprintf("%s-%s", hostname, uuidRegex)
matched, err := regexp.MatchString(regexPattern, se.collectorName)
require.NoError(t, err)
assert.True(t, matched)
require.Equal(t, hostname, se.collectorName)

require.NoError(t, se.Shutdown(context.Background()))
}
Expand All @@ -1394,6 +1374,8 @@ func TestWatchCredentialKey(t *testing.T) {

go func() {
time.Sleep(time.Millisecond * 100)
se.credsNotifyLock.Lock()
defer se.credsNotifyLock.Unlock()
se.registrationInfo.CollectorCredentialKey = "test-credential-key"
close(se.credsNotifyUpdate)
}()
Expand Down
1 change: 0 additions & 1 deletion pkg/extension/sumologicextension/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ go 1.18

require (
github.com/cenkalti/backoff/v4 v4.2.0
github.com/google/uuid v1.3.0
github.com/hashicorp/go-multierror v1.1.1
github.com/mitchellh/go-ps v1.0.0
github.com/shirou/gopsutil/v3 v3.22.12
Expand Down
2 changes: 0 additions & 2 deletions pkg/extension/sumologicextension/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,6 @@ github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk=
github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw=
github.com/hashicorp/consul/api v1.13.0/go.mod h1:ZlVrynguJKcYr54zGaDbaL3fOvKC9m72FhPvA8T35KQ=
Expand Down