From 03a0c8fbad1fe33932b15b828583d096a6955b43 Mon Sep 17 00:00:00 2001 From: Adam Charrett Date: Mon, 18 Mar 2024 21:05:14 +0000 Subject: [PATCH 1/2] [exporter/awss3] Make file format suffix configurable --- exporter/awss3exporter/README.md | 1 + exporter/awss3exporter/config.go | 4 +-- exporter/awss3exporter/config_test.go | 3 ++- exporter/awss3exporter/exporter.go | 2 +- exporter/awss3exporter/marshaler.go | 4 +-- exporter/awss3exporter/marshaler_test.go | 6 ++--- exporter/awss3exporter/s3_writer.go | 8 ++++-- exporter/awss3exporter/s3_writer_test.go | 28 ++++++++++++++++++++ exporter/awss3exporter/testdata/default.yaml | 1 + 9 files changed, 46 insertions(+), 11 deletions(-) diff --git a/exporter/awss3exporter/README.md b/exporter/awss3exporter/README.md index c6f13cc4fc932..594370921fc01 100644 --- a/exporter/awss3exporter/README.md +++ b/exporter/awss3exporter/README.md @@ -32,6 +32,7 @@ The following exporter configuration parameters are supported. | `file_prefix` | file prefix defined by user | | | `marshaler` | marshaler used to produce output data | `otlp_json` | | `encoding` | Encoding extension to use to marshal data. Overrides the `marshaler` configuration option if set. | | +| `encoding_file_extension` | file format extension suffix when using the `encoding` configuration option. May be left empty for no suffix to be appended. | | | `endpoint` | overrides the endpoint used by the exporter instead of constructing it from `region` and `s3_bucket` | | | `s3_force_path_style` | [set this to `true` to force the request to use path-style addressing](http://docs.aws.amazon.com/AmazonS3/latest/dev/VirtualHosting.html) | false | | `disable_ssl` | set this to `true` to disable SSL when sending requests | false | diff --git a/exporter/awss3exporter/config.go b/exporter/awss3exporter/config.go index 82397f67ab35f..276d73f357b11 100644 --- a/exporter/awss3exporter/config.go +++ b/exporter/awss3exporter/config.go @@ -40,9 +40,9 @@ type Config struct { S3Uploader S3UploaderConfig `mapstructure:"s3uploader"` MarshalerName MarshalerType `mapstructure:"marshaler"` - FileFormat string `mapstructure:"file_format"` // Encoding to apply. If present, overrides the marshaler configuration option. - Encoding *component.ID `mapstructure:"encoding"` + Encoding *component.ID `mapstructure:"encoding"` + EncodingFileExtension string `mapstructure:"encoding_file_extension"` } func (c *Config) Validate() error { diff --git a/exporter/awss3exporter/config_test.go b/exporter/awss3exporter/config_test.go index dd253b5a9dc5c..8638aa27076b0 100644 --- a/exporter/awss3exporter/config_test.go +++ b/exporter/awss3exporter/config_test.go @@ -32,7 +32,8 @@ func TestLoadConfig(t *testing.T) { encoding := component.MustNewIDWithName("foo", "bar") assert.Equal(t, e, &Config{ - Encoding: &encoding, + Encoding: &encoding, + EncodingFileExtension: "baz", S3Uploader: S3UploaderConfig{ Region: "us-east-1", S3Bucket: "foo", diff --git a/exporter/awss3exporter/exporter.go b/exporter/awss3exporter/exporter.go index 0650886dfdd0d..f4524ee40be08 100644 --- a/exporter/awss3exporter/exporter.go +++ b/exporter/awss3exporter/exporter.go @@ -39,7 +39,7 @@ func (e *s3Exporter) start(_ context.Context, host component.Host) error { var m marshaler var err error if e.config.Encoding != nil { - if m, err = newMarshalerFromEncoding(e.config.Encoding, host, e.logger); err != nil { + if m, err = newMarshalerFromEncoding(e.config.Encoding, e.config.EncodingFileExtension, host, e.logger); err != nil { return err } } else { diff --git a/exporter/awss3exporter/marshaler.go b/exporter/awss3exporter/marshaler.go index 6054c6977bf9a..57ac6455d8230 100644 --- a/exporter/awss3exporter/marshaler.go +++ b/exporter/awss3exporter/marshaler.go @@ -25,7 +25,7 @@ var ( ErrUnknownMarshaler = errors.New("unknown marshaler") ) -func newMarshalerFromEncoding(encoding *component.ID, host component.Host, logger *zap.Logger) (marshaler, error) { +func newMarshalerFromEncoding(encoding *component.ID, fileFormat string, host component.Host, logger *zap.Logger) (marshaler, error) { marshaler := &s3Marshaler{logger: logger} e, ok := host.GetExtensions()[*encoding] if !ok { @@ -35,7 +35,7 @@ func newMarshalerFromEncoding(encoding *component.ID, host component.Host, logge marshaler.logsMarshaler, _ = e.(plog.Marshaler) marshaler.metricsMarshaler, _ = e.(pmetric.Marshaler) marshaler.tracesMarshaler, _ = e.(ptrace.Marshaler) - marshaler.fileFormat = encoding.String() + marshaler.fileFormat = fileFormat return marshaler, nil } diff --git a/exporter/awss3exporter/marshaler_test.go b/exporter/awss3exporter/marshaler_test.go index 09f1e8fb59823..9a56d83d1f6fa 100644 --- a/exporter/awss3exporter/marshaler_test.go +++ b/exporter/awss3exporter/marshaler_test.go @@ -90,13 +90,13 @@ func TestMarshalerFromEncoding(t *testing.T) { host := hostWithExtensions{ encoding: encodingExtension{}, } - m, err := newMarshalerFromEncoding(&id, host, zap.NewNop()) + m, err := newMarshalerFromEncoding(&id, "myext", host, zap.NewNop()) assert.NoError(t, err) require.NotNil(t, m) - assert.Equal(t, "foo", m.format()) + assert.Equal(t, "myext", m.format()) } { - m, err := newMarshalerFromEncoding(&id, componenttest.NewNopHost(), zap.NewNop()) + m, err := newMarshalerFromEncoding(&id, "", componenttest.NewNopHost(), zap.NewNop()) assert.EqualError(t, err, `unknown encoding "foo"`) require.Nil(t, m) } diff --git a/exporter/awss3exporter/s3_writer.go b/exporter/awss3exporter/s3_writer.go index 1518bb48ecf01..38376c45605d4 100644 --- a/exporter/awss3exporter/s3_writer.go +++ b/exporter/awss3exporter/s3_writer.go @@ -40,11 +40,15 @@ func randomInRange(low, hi int) int { return low + rand.Intn(hi-low) } -func getS3Key(time time.Time, keyPrefix string, partition string, filePrefix string, metadata string, fileformat string, compression configcompression.Type) string { +func getS3Key(time time.Time, keyPrefix string, partition string, filePrefix string, metadata string, fileFormat string, compression configcompression.Type) string { timeKey := getTimeKey(time, partition) randomID := randomInRange(100000000, 999999999) + suffix := "" + if fileFormat != "" { + suffix = "." + fileFormat + } - s3Key := keyPrefix + "/" + timeKey + "/" + filePrefix + metadata + "_" + strconv.Itoa(randomID) + "." + fileformat + s3Key := keyPrefix + "/" + timeKey + "/" + filePrefix + metadata + "_" + strconv.Itoa(randomID) + suffix // add ".gz" extension to files if compression is enabled if compression == configcompression.TypeGzip { diff --git a/exporter/awss3exporter/s3_writer_test.go b/exporter/awss3exporter/s3_writer_test.go index 1da692ba92fa2..6df1998b07d5d 100644 --- a/exporter/awss3exporter/s3_writer_test.go +++ b/exporter/awss3exporter/s3_writer_test.go @@ -41,6 +41,20 @@ func TestS3Key(t *testing.T) { assert.Equal(t, true, matched) } +func TestS3KeyEmptyFileFormat(t *testing.T) { + const layout = "2006-01-02" + + tm, err := time.Parse(layout, "2022-06-05") + + assert.NoError(t, err) + require.NotNil(t, tm) + + re := regexp.MustCompile(`keyprefix/year=2022/month=06/day=05/hour=00/minute=00/fileprefixlogs_([0-9]+)`) + s3Key := getS3Key(tm, "keyprefix", "minute", "fileprefix", "logs", "", "") + matched := re.MatchString(s3Key) + assert.Equal(t, true, matched) +} + func TestS3KeyOfCompressedFile(t *testing.T) { const layout = "2006-01-02" @@ -55,6 +69,20 @@ func TestS3KeyOfCompressedFile(t *testing.T) { assert.Equal(t, true, matched) } +func TestS3KeyOfCompressedFileEmptyFileFormat(t *testing.T) { + const layout = "2006-01-02" + + tm, err := time.Parse(layout, "2022-06-05") + + assert.NoError(t, err) + require.NotNil(t, tm) + + re := regexp.MustCompile(`keyprefix/year=2022/month=06/day=05/hour=00/minute=00/fileprefixlogs_([0-9]+).gz`) + s3Key := getS3Key(tm, "keyprefix", "minute", "fileprefix", "logs", "", "gzip") + matched := re.MatchString(s3Key) + assert.Equal(t, true, matched) +} + func TestGetSessionConfigWithEndpoint(t *testing.T) { const endpoint = "https://endpoint.com" const region = "region" diff --git a/exporter/awss3exporter/testdata/default.yaml b/exporter/awss3exporter/testdata/default.yaml index ddbf714395727..656db12122ea3 100644 --- a/exporter/awss3exporter/testdata/default.yaml +++ b/exporter/awss3exporter/testdata/default.yaml @@ -4,6 +4,7 @@ receivers: exporters: awss3: encoding: "foo/bar" + encoding_file_extension: "baz" s3uploader: s3_bucket: "foo" region: 'us-east-1' From a14c8b6aec31e6ade4394b906c90ba53aedb56e1 Mon Sep 17 00:00:00 2001 From: Adam Charrett Date: Mon, 18 Mar 2024 21:12:08 +0000 Subject: [PATCH 2/2] Create awss3_encoding_file_extension.yaml --- .chloggen/awss3_encoding_file_extension.yaml | 27 ++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 .chloggen/awss3_encoding_file_extension.yaml diff --git a/.chloggen/awss3_encoding_file_extension.yaml b/.chloggen/awss3_encoding_file_extension.yaml new file mode 100644 index 0000000000000..6c3e57739426e --- /dev/null +++ b/.chloggen/awss3_encoding_file_extension.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# 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: awss3exporter + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "Add support for specifying the file extension for files uploaded to S3 when using an encoding extension." + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [31818] + +# (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: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [user]