Support configure aws endpoint url (#21917)

**Description:** <Describe what has changed.>  The AWS S3 exporter configuration currently doesn't allow customization of the S3 endpoint, which can be troublesome during local development. To improve flexibility, the exporter now supports specifying a custom endpoint during session creation. **Link to tracking Issue:** <Issue number if applicable> #21833 **Testing:** <Describe what testing was performed and which tests were added.> - UT with endpoint configured **Documentation:** <Describe the documentation added.> update README with the new field --------- Signed-off-by: guyfried <[email protected]> Co-authored-by: Antoine Toulme <[email protected]> Co-authored-by: Pablo Baeyens <[email protected]>
open-telemetry · Aug 1, 2023 · 475ce03 · 475ce03
1 parent a2bc3eb
commit 475ce03
Show file tree

Hide file tree

Showing 9 changed files with 146 additions and 12 deletions.
diff --git a/.chloggen/support-configure-aws-endpoint.yaml b/.chloggen/support-configure-aws-endpoint.yaml
@@ -0,0 +1,12 @@
+# 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. filelogreceiver)
+component: exporter/awss3exporter
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Allow custom endpoints to be configured for exporting spans
+
+# One or more tracking issues related to the change
+issues: [21833]
+
diff --git a/exporter/awss3exporter/README.md b/exporter/awss3exporter/README.md
@@ -21,14 +21,15 @@ This exporter targets to support proto/json format.
 
 The following exporter configuration parameters are supported. 
 
-| Name           | Description                                           | Default  |
-|:---------------|:------------------------------------------------------|----------|
-| `region`       | AWS region.                                           |          |
-| `s3_bucket`    | S3 bucket                                             |          |
-| `s3_prefix`    | prefix for the S3 key (root directory inside bucket). |          |
-| `s3_partition` | time granularity of S3 key: hour or minute            | "minute" |
-| `file_prefix`  | file prefix defined by user                           |          |
-| `marshaler`    | marshaler used to produce output data otlp_json       |          |
+| Name           | Description                                                                                          | Default     |
+|:---------------|:-----------------------------------------------------------------------------------------------------|-------------|
+| `region`       | AWS region.                                                                                          | "us-east-1" |
+| `s3_bucket`    | S3 bucket                                                                                            |             |
+| `s3_prefix`    | prefix for the S3 key (root directory inside bucket).                                                |             |
+| `s3_partition` | time granularity of S3 key: hour or minute                                                           | "minute"    |
+| `file_prefix`  | file prefix defined by user                                                                          |             |
+| `marshaler`    | marshaler used to produce output data otlp_json                                                      |             |
+| `endpoint`     | overrides the endpoint used by the exporter instead of constructing it from `region` and `s3_bucket` |             |
 
 # Example Configuration
 

diff --git a/exporter/awss3exporter/config.go b/exporter/awss3exporter/config.go
@@ -3,6 +3,12 @@
 
 package awss3exporter // import "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/awss3exporter"
 
+import (
+	"errors"
+
+	"go.uber.org/multierr"
+)
+
 // S3UploaderConfig contains aws s3 uploader related config to controls things
 // like bucket, prefix, batching, connections, retries, etc.
 type S3UploaderConfig struct {
@@ -11,6 +17,7 @@ type S3UploaderConfig struct {
 	S3Prefix    string `mapstructure:"s3_prefix"`
 	S3Partition string `mapstructure:"s3_partition"`
 	FilePrefix  string `mapstructure:"file_prefix"`
+	Endpoint    string `mapstructure:"endpoint"`
 }
 
 type MarshalerType string
@@ -26,3 +33,14 @@ type Config struct {
 
 	FileFormat string `mapstructure:"file_format"`
 }
+
+func (c *Config) Validate() error {
+	var errs error
+	if c.S3Uploader.Region == "" {
+		errs = multierr.Append(errs, errors.New("region is required"))
+	}
+	if c.S3Uploader.S3Bucket == "" {
+		errs = multierr.Append(errs, errors.New("bucket is required"))
+	}
+	return errs
+}
diff --git a/exporter/awss3exporter/config_test.go b/exporter/awss3exporter/config_test.go
@@ -4,13 +4,15 @@
 package awss3exporter
 
 import (
+	"errors"
 	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/otelcol/otelcoltest"
+	"go.uber.org/multierr"
 )
 
 func TestLoadConfig(t *testing.T) {
@@ -29,6 +31,7 @@ func TestLoadConfig(t *testing.T) {
 		&Config{
 			S3Uploader: S3UploaderConfig{
 				Region:      "us-east-1",
+				S3Bucket:    "foo",
 				S3Partition: "minute",
 			},
 			MarshalerName: "otlp_json",
@@ -57,8 +60,67 @@ func TestConfig(t *testing.T) {
 				S3Bucket:    "foo",
 				S3Prefix:    "bar",
 				S3Partition: "minute",
+				Endpoint:    "http://endpoint.com",
 			},
 			MarshalerName: "otlp_json",
 		},
 	)
 }
+
+func TestConfig_Validate(t *testing.T) {
+	tests := []struct {
+		name        string
+		config      *Config
+		errExpected error
+	}{
+		{
+			name: "valid",
+			config: func() *Config {
+				c := createDefaultConfig().(*Config)
+				c.S3Uploader.Region = "foo"
+				c.S3Uploader.S3Bucket = "bar"
+				c.S3Uploader.Endpoint = "http://example.com"
+				return c
+			}(),
+			errExpected: nil,
+		},
+		{
+			name: "missing all",
+			config: func() *Config {
+				c := createDefaultConfig().(*Config)
+				c.S3Uploader.Region = ""
+				return c
+			}(),
+			errExpected: multierr.Append(errors.New("region is required"),
+				errors.New("bucket is required")),
+		},
+		{
+			name: "endpoint and region",
+			config: func() *Config {
+				c := createDefaultConfig().(*Config)
+				c.S3Uploader.Endpoint = "http://example.com"
+				c.S3Uploader.Region = "foo"
+				return c
+			}(),
+			errExpected: errors.New("bucket is required"),
+		},
+		{
+			name: "endpoint and bucket",
+			config: func() *Config {
+				c := createDefaultConfig().(*Config)
+				c.S3Uploader.Endpoint = "http://example.com"
+				c.S3Uploader.S3Bucket = "foo"
+				c.S3Uploader.Region = ""
+				return c
+			}(),
+			errExpected: errors.New("region is required"),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := tt.config.Validate()
+			require.Equal(t, tt.errExpected, err)
+		})
+	}
+}
diff --git a/exporter/awss3exporter/go.mod b/exporter/awss3exporter/go.mod
@@ -10,6 +10,7 @@ require (
 	go.opentelemetry.io/collector/consumer v0.82.0
 	go.opentelemetry.io/collector/exporter v0.82.0
 	go.opentelemetry.io/collector/pdata v1.0.0-rcv0014
+	go.uber.org/multierr v1.11.0
 	go.uber.org/zap v1.24.0
 )
 
@@ -80,7 +81,6 @@ require (
 	go.opentelemetry.io/otel/trace v1.16.0 // indirect
 	go.opentelemetry.io/proto/otlp v0.19.0 // indirect
 	go.uber.org/atomic v1.10.0 // indirect
-	go.uber.org/multierr v1.11.0 // indirect
 	golang.org/x/net v0.12.0 // indirect
 	golang.org/x/sys v0.10.0 // indirect
 	golang.org/x/text v0.11.0 // indirect

diff --git a/exporter/awss3exporter/s3_writer.go b/exporter/awss3exporter/s3_writer.go
@@ -46,6 +46,19 @@ func getS3Key(time time.Time, keyPrefix string, partition string, filePrefix str
 	return s3Key
 }
 
+func getSessionConfig(config *Config) *aws.Config {
+	sessionConfig := &aws.Config{
+		Region: aws.String(config.S3Uploader.Region),
+	}
+
+	endpoint := config.S3Uploader.Endpoint
+	if endpoint != "" {
+		sessionConfig.Endpoint = aws.String(endpoint)
+	}
+
+	return sessionConfig
+}
+
 func (s3writer *s3Writer) writeBuffer(_ context.Context, buf []byte, config *Config, metadata string, format string) error {
 	now := time.Now()
 	key := getS3Key(now,
@@ -55,9 +68,8 @@ func (s3writer *s3Writer) writeBuffer(_ context.Context, buf []byte, config *Con
 	// create a reader from data data in memory
 	reader := bytes.NewReader(buf)
 
-	sess, err := session.NewSession(&aws.Config{
-		Region: aws.String(config.S3Uploader.Region)},
-	)
+	sessionConfig := getSessionConfig(config)
+	sess, err := session.NewSession(sessionConfig)
 
 	if err != nil {
 		return err

diff --git a/exporter/awss3exporter/s3_writer_test.go b/exporter/awss3exporter/s3_writer_test.go
@@ -8,6 +8,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/aws/aws-sdk-go/aws"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -39,3 +40,29 @@ func TestS3Key(t *testing.T) {
 	matched := re.MatchString(s3Key)
 	assert.Equal(t, true, matched)
 }
+
+func TestGetSessionConfigWithEndpoint(t *testing.T) {
+	const endpoint = "https://endpoint.com"
+	const region = "region"
+	config := &Config{
+		S3Uploader: S3UploaderConfig{
+			Region:   region,
+			Endpoint: endpoint,
+		},
+	}
+	sessionConfig := getSessionConfig(config)
+	assert.Equal(t, sessionConfig.Endpoint, aws.String(endpoint))
+	assert.Equal(t, sessionConfig.Region, aws.String(region))
+}
+
+func TestGetSessionConfigNoEndpoint(t *testing.T) {
+	const region = "region"
+	config := &Config{
+		S3Uploader: S3UploaderConfig{
+			Region: region,
+		},
+	}
+	sessionConfig := getSessionConfig(config)
+	assert.Empty(t, sessionConfig.Endpoint)
+	assert.Equal(t, sessionConfig.Region, aws.String(region))
+}
diff --git a/exporter/awss3exporter/testdata/config.yaml b/exporter/awss3exporter/testdata/config.yaml
@@ -8,6 +8,7 @@ exporters:
         s3_bucket: 'foo'
         s3_prefix: 'bar'
         s3_partition: 'minute'
+        endpoint: "http://endpoint.com"
 
 processors:
   nop:

diff --git a/exporter/awss3exporter/testdata/default.yaml b/exporter/awss3exporter/testdata/default.yaml
@@ -4,6 +4,7 @@ receivers:
 exporters:
   awss3:
     s3uploader:
+      s3_bucket: "foo"
       region: 'us-east-1'
       s3_partition: 'minute'