From a57036815b8de153133ece724a4dcf5a56855d5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 14 May 2021 18:54:07 +0200 Subject: [PATCH] Add support for access token and cleanup other configs (#55) * Add support for access token and cleanup other configs * Update README.md * Apply suggestions from code review Co-authored-by: Tyler Yahn Co-authored-by: Owais Lone Co-authored-by: Tyler Yahn --- CHANGELOG.md | 21 +++++++- README.md | 11 ++++ distro/config.go | 54 +++++++++---------- distro/config_test.go | 35 ++++-------- distro/helpers_test.go | 31 +++++++++++ distro/otel.go | 10 +++- distro/otel_test.go | 118 ++++++++++++++++++++++++++++++++--------- 7 files changed, 199 insertions(+), 81 deletions(-) create mode 100644 distro/helpers_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index b9e875a58..fd3ba2dbe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,12 +8,31 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Added + +- Add support for setting the [Splunk's organization access token](https://docs.splunk.com/observability/admin/authentication-tokens/org-tokens.html) + using the `SPLUNK_ACCESS_TOKEN` environmental variable or `distro.WithAccessToken` option. + It allows exporters sending data directly to the Splunk back-end. + To do so, the `OTEL_EXPORTER_JAEGER_ENDPOINT` or `distro.WithEndpoint` must be set + with Splunk back-end ingest endpoint URL: `https://ingest..signalfx.com/v2/trace`. + +### Changed + +- The default Jaeger exporter URL is now loaded by + [`go.opentelemetry.io/otel/exporters/trace/jaeger`](https://pkg.go.dev/go.opentelemetry.io/otel/exporters/trace/jaeger) + module. +- Applying `distro.WithEndpoint("")` results in no operation. + +### Removed + +- Remove `SIGNALFX_ENDPOINT_URL` environmental variable, use `OTEL_EXPORTER_JAEGER_ENDPOINT` instead. + ## [0.2.0] - 2021-04-27 The primary change of this release is updating the dependency of `go.opentelemetry.io/otel*` packages from [`v0.19.0`][otel-v0.19.0] to [`v0.20.0`][otel-v0.20.0] and similarly `go.opentelemetry.io/contrib*` package from [`v0.19.0`][contrib-v0.19.0] to [`v0.20.0`][contrib-v0.20.0]. This includes [a fix](https://github.com/open-telemetry/opentelemetry-go/pull/1830) in the Jaeger exporter. This fix removes the duplicate batching that the exporter implemented. -Now the `BatchSpanProcessor` that `distro` configures by default will not experience an impedence mismatch with this duplicate batching. +Now the `BatchSpanProcessor` that `distro` configures by default will not experience an impedance mismatch with this duplicate batching. ### Changed diff --git a/README.md b/README.md index 00e83d97c..95872019f 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,17 @@ func main() { Documentation on how to manually instrument a Go application is available [here](https://opentelemetry.io/docs/go/getting-started/). +## Splunk specific configuration + +| Environment variable | Option | Default value | Description | +| ------------------------- | -------------------| -------------- | ---------------------------------------------------------------------- | +| `SPLUNK_ACCESS_TOKEN` | `WithAccessToken` | | The [Splunk's organization access token](https://docs.splunk.com/observability/admin/authentication-tokens/org-tokens.html). [[1](#cfg1)] | + +[1]: The [Splunk's organization access token](https://docs.splunk.com/observability/admin/authentication-tokens/org-tokens.html) +allows exporters sending data directly to the [Splunk Observability Cloud](https://dev.splunk.com/observability/docs/apibasics/api_list/). +To do so, the `OTEL_EXPORTER_JAEGER_ENDPOINT` or `contrib.WithEndpoint` must be set +with Splunk back-end ingest endpoint URL: `https://ingest..signalfx.com/v2/trace`. + ## Splunk specific instrumentations - [`splunkhttp`](./instrumentation/net/http/splunkhttp) diff --git a/distro/config.go b/distro/config.go index 2ae1fc68e..7553e8184 100644 --- a/distro/config.go +++ b/distro/config.go @@ -22,17 +22,11 @@ import ( // Environment variable keys that set values of the configuration. const ( - serviceNameKey = "SIGNALFX_SERVICE_NAME" - endpointURLKey = "SIGNALFX_ENDPOINT_URL" - // TODO: support these - // accessTokenKey = "SIGNALFX_ACCESS_TOKEN" - // spanTagsKey = "SIGNALFX_SPAN_TAGS" - // recordedValueMaxLengthKey = "SIGNALFX_RECORDED_VALUE_MAX_LENGTH" + accessTokenKey = "SPLUNK_ACCESS_TOKEN" ) // config is the configuration used to create and operate an SDK. type config struct { - ServiceName string AccessToken string Endpoint string } @@ -40,14 +34,12 @@ type config struct { // newConfig returns a validated config with Splunk defaults. func newConfig(opts ...Option) (*config, error) { c := &config{ - ServiceName: envOr(serviceNameKey, "unnamed-go-service"), - Endpoint: envOr(endpointURLKey, "http://localhost:9080/v1/trace"), + AccessToken: envOr(accessTokenKey, ""), } for _, o := range opts { - o(c) + o.apply(c) } - if err := c.Validate(); err != nil { return nil, err } @@ -58,18 +50,15 @@ func newConfig(opts ...Option) (*config, error) { func (c config) Validate() error { var errs []string - if c.ServiceName == "" { - errs = append(errs, "empty service name") - } - - if _, err := url.Parse(c.Endpoint); err != nil { - errs = append(errs, "invalid endpoint: %s", err.Error()) + if c.Endpoint != "" { + if _, err := url.Parse(c.Endpoint); err != nil { + errs = append(errs, "invalid endpoint: %s", err.Error()) + } } if len(errs) > 0 { return fmt.Errorf("invalid config: %v", errs) } - return nil } @@ -84,19 +73,30 @@ func envOr(key, alt string) string { } // Option sets a config setting value. -type Option func(*config) +type Option interface { + apply(*config) +} -// WithServiceName configures the service name that collected telemetry is -// associated with. -func WithServiceName(name string) Option { - return func(c *config) { - c.ServiceName = name - } +// optionFunc is a functional option implementation for Option interface. +type optionFunc func(*config) + +func (fn optionFunc) apply(c *config) { + fn(c) } // WithEndpoint configures the endpoint telemetry is sent to. +// Setting empty string results in no operation. func WithEndpoint(endpoint string) Option { - return func(c *config) { + return optionFunc(func(c *config) { c.Endpoint = endpoint - } + }) +} + +// WithAccessToken configures the authentication token +// allowing exporters to send data directly to a Splunk back-end. +// Setting empty string results in no operation. +func WithAccessToken(accessToken string) Option { + return optionFunc(func(c *config) { + c.AccessToken = accessToken + }) } diff --git a/distro/config_test.go b/distro/config_test.go index 2fca400cc..b863836bd 100644 --- a/distro/config_test.go +++ b/distro/config_test.go @@ -15,7 +15,6 @@ package distro import ( - "os" "testing" "github.com/stretchr/testify/assert" @@ -42,32 +41,23 @@ type ConfigFieldTest struct { var ConfigurationTests = []*ConfigFieldTest{ { - Name: "ServiceName", + Name: "AccessToken", ValueFunc: func(c *config) interface{} { - return c.ServiceName + return c.AccessToken }, - DefaultValue: "unnamed-go-service", + DefaultValue: "", EnvironmentTests: []KeyValue{ - {Key: serviceNameKey, Value: "service"}, + {Key: accessTokenKey, Value: "secret"}, }, OptionTests: []OptionTest{ { Name: "valid name", Options: []Option{ - WithServiceName("test-service"), + WithAccessToken("secret"), }, AssertionFunc: func(t *testing.T, c *config, e error) { assert.NoError(t, e) - assert.Equal(t, "test-service", c.ServiceName) - }, - }, - { - Name: "invalid name", - Options: []Option{ - WithServiceName(""), - }, - AssertionFunc: func(t *testing.T, c *config, e error) { - assert.Error(t, e) + assert.Equal(t, "secret", c.AccessToken) }, }, }, @@ -77,10 +67,7 @@ var ConfigurationTests = []*ConfigFieldTest{ ValueFunc: func(c *config) interface{} { return c.Endpoint }, - DefaultValue: "http://localhost:9080/v1/trace", - EnvironmentTests: []KeyValue{ - {Key: endpointURLKey, Value: "https://localhost/"}, - }, + DefaultValue: "", OptionTests: []OptionTest{ { Name: "valid URL", @@ -130,12 +117,8 @@ func TestConfig(t *testing.T) { func testEnvironmentOverrides(t *testing.T, tc *ConfigFieldTest) { for _, ev := range tc.EnvironmentTests { func(key, val string) { - if v, ok := os.LookupEnv(key); ok { - defer func() { os.Setenv(key, v) }() - } else { - defer func() { os.Unsetenv(key) }() - } - os.Setenv(key, val) + revert := Setenv(key, val) + defer revert() c, err := newConfig() if !assert.NoError(t, err) { diff --git a/distro/helpers_test.go b/distro/helpers_test.go new file mode 100644 index 000000000..53a929f37 --- /dev/null +++ b/distro/helpers_test.go @@ -0,0 +1,31 @@ +// Copyright Splunk Inc. +// +// 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 distro + +import "os" + +// Setenv sets the value of the environment variable named by the key. +// It returns a function that rollbacks the setting. +func Setenv(key, val string) func() { + valSnapshot, ok := os.LookupEnv(key) + os.Setenv(key, val) + return func() { + if ok { + os.Setenv(key, valSnapshot) + } else { + os.Unsetenv(key) + } + } +} diff --git a/distro/otel.go b/distro/otel.go index 03c7a2271..cd8af2e64 100644 --- a/distro/otel.go +++ b/distro/otel.go @@ -51,7 +51,15 @@ func Run(opts ...Option) (SDK, error) { return SDK{}, err } - opt := jaeger.WithCollectorEndpoint(jaeger.WithEndpoint(c.Endpoint)) + var jeagerOpts []jaeger.CollectorEndpointOption + if c.Endpoint != "" { + jeagerOpts = append(jeagerOpts, jaeger.WithEndpoint(c.Endpoint)) + } + if c.AccessToken != "" { + jeagerOpts = append(jeagerOpts, jaeger.WithUsername("auth"), jaeger.WithPassword(c.AccessToken)) + } + + opt := jaeger.WithCollectorEndpoint(jeagerOpts...) exp, err := jaeger.NewRawExporter(opt) if err != nil { return SDK{}, err diff --git a/distro/otel_test.go b/distro/otel_test.go index 68cac88a5..b799eb714 100644 --- a/distro/otel_test.go +++ b/distro/otel_test.go @@ -28,37 +28,103 @@ import ( "go.opentelemetry.io/otel/attribute" ) -// TestRun is a smoke test that ensures that traces are sent using thrift protocol. +// TestRun is a collection of sanity tests that ensure that traces are sent using thrift protocol. func TestRun(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() + testCases := []struct { + desc string + setupFn func(t *testing.T, url string) (distro.SDK, error) + assertFn func(t *testing.T, req *http.Request) + }{ + { + desc: "WithEndpoint", + setupFn: func(t *testing.T, url string) (distro.SDK, error) { + return distro.Run(distro.WithEndpoint(url)) + }, + assertFn: func(t *testing.T, got *http.Request) { + assert.Equal(t, "application/x-thrift", got.Header.Get("Content-type"), "should send thrift formatted trace") + }, + }, + { + desc: "OTEL_EXPORTER_JAEGER_ENDPOINT", + setupFn: func(t *testing.T, url string) (distro.SDK, error) { + clearFn := distro.Setenv("OTEL_EXPORTER_JAEGER_ENDPOINT", url) + t.Cleanup(clearFn) + return distro.Run() + }, + assertFn: func(t *testing.T, got *http.Request) { + assert.Equal(t, "application/x-thrift", got.Header.Get("Content-type"), "should send thrift formatted trace") + }, + }, + { + desc: "WithEndpoint and WithAccessToken", + setupFn: func(t *testing.T, url string) (distro.SDK, error) { + return distro.Run(distro.WithEndpoint(url), distro.WithAccessToken("my-token")) + }, + assertFn: func(t *testing.T, got *http.Request) { + assert.Equal(t, "application/x-thrift", got.Header.Get("Content-type"), "should send thrift formatted trace") + user, pass, ok := got.BasicAuth() + if !ok { + assert.Fail(t, "should have Basic Authentication headers") + return + } + assert.Equal(t, "auth", user, "should have proper username") + assert.Equal(t, "my-token", pass, "should use the provided token as passowrd") + }, + }, + { + desc: "OTEL_EXPORTER_JAEGER_ENDPOINT and SPLUNK_ACCESS_TOKEN", + setupFn: func(t *testing.T, url string) (distro.SDK, error) { + clearFn := distro.Setenv("OTEL_EXPORTER_JAEGER_ENDPOINT", url) + t.Cleanup(clearFn) + clearFn = distro.Setenv("SPLUNK_ACCESS_TOKEN", "my-token") + t.Cleanup(clearFn) + return distro.Run() + }, + assertFn: func(t *testing.T, got *http.Request) { + assert.Equal(t, "application/x-thrift", got.Header.Get("Content-type"), "should send thrift formatted trace") + user, pass, ok := got.BasicAuth() + if !ok { + assert.Fail(t, "should have Basic Authentication headers") + return + } + assert.Equal(t, "auth", user, "should have proper username") + assert.Equal(t, "my-token", pass, "should use the provided token as passowrd") + }, + }, + } + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() - // HTTP endpoint where a trace is sent - reqCh := make(chan *http.Request, 1) - srv := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { - reqCh <- r - })) - defer srv.Close() + // HTTP endpoint where a trace is sent + reqCh := make(chan *http.Request, 1) + srv := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + reqCh <- r + })) + defer srv.Close() - // setup tracer - sdk, err := distro.Run(distro.WithEndpoint(srv.URL)) - require.NoError(t, err, "should configure tracing") + // setup tracer + sdk, err := tc.setupFn(t, srv.URL) + require.NoError(t, err, "should configure tracing") - // create a sample span - _, span := otel.Tracer("distro/otel_test").Start(ctx, "TestRun") - span.SetAttributes(attribute.Key("ex.com/foo").String("bar")) - span.AddEvent("working") - span.End() + // create a sample span + _, span := otel.Tracer("distro/otel_test").Start(ctx, "TestRun") + span.SetAttributes(attribute.Key("ex.com/foo").String("bar")) + span.AddEvent("working") + span.End() - // shutdown tracer - this should send the trace - err = sdk.Shutdown(ctx) - require.NoError(t, err, "should finish tracing") + // shutdown tracer - this should send the trace + err = sdk.Shutdown(ctx) + require.NoError(t, err, "should finish tracing") - // assert that the span has been received - select { - case <-ctx.Done(): - require.Fail(t, "test timeout out") - case got := <-reqCh: - assert.Equal(t, "application/x-thrift", got.Header.Get("Content-type"), "should send thrift formatted trace") + // assert that the span has been received + select { + case <-ctx.Done(): + require.Fail(t, "test timeout out") + case got := <-reqCh: + tc.assertFn(t, got) + } + }) } }