Creating AWS S3 Config Source #4575

Aditya-Gollapudi · 2021-08-09T23:04:26Z

Moved this from collector to contrib

Note: Please don't review anything in the configprovider folder - that will all be merged in a different PR (coming from Paolo) The only stuff from this that will be merged is in the s3configsource folder. This PR was mostly submitted to try to parallelize some of the reviewing

Description: Added a Config Source for S3 inline with the new config source changes that have been made

Testing: So far there is only a core test, but I will continue to add them - this is something of a draft. Also please note that we expect the configprovider folder to be merged in at some point later on but that won't live here - where exactly everything is to be merged is still up in the air

@pjanotti

Link to old PR: open-telemetry/opentelemetry-collector#3738

Aditya test old

Removed a local testing file

…mmenting

…y fatally logging

…try-collector-contrib into main

bogdandrutu · 2021-08-10T16:31:56Z

Please fix the build:

# github.com/open-telemetry/opentelemetry-collector-contrib/exporter/awsemfexporter
exporter\awsemfexporter\grouped_metric.go:20:2: json redeclared as imported package name
	previous declaration at exporter\awsemfexporter\grouped_metric.go:18:2
FAIL	github.com/open-telemetry/opentelemetry-collector-contrib/cmd/otelcontribcol [build failed]
ok  	github.com/open-telemetry/opentelemetry-collector-contrib/internal/version	0.040s [no tests to run]
FAIL

anuraaga

Is it possible to use an abstraction like vfs to support all clouds?

https://github.com/C2FO/vfs

Reading config files doesn't seem like an extremely high-scale operation or anything that might benefit performance-wise from being "closer to metal" using the raw cloud sdks so maybe it's practical.

anuraaga · 2021-08-11T06:25:08Z

internal/aws/awss3configsource/config.go

+package awss3configsource
+
+import (
+	splunkprovider "github.com/signalfx/splunk-otel-collector"


It seems quite odd for the upstream collector to depend on a downstream distro, it's like a circular dependency

Splunk is currently submitting a string of PRs into Collector - the plan is to have this merged in - in fact in the go.mod file I replace this reference with a local reference to the config provider folder

bogdandrutu · 2021-08-11T16:15:06Z

internal/aws/awss3configsource/go.mod

+	github.com/knadh/koanf v1.2.0
+	github.com/signalfx/splunk-otel-collector v0.0.0-00010101000000-000000000000
+	github.com/stretchr/testify v1.7.0
+	go.opentelemetry.io/collector v0.31.0


Please use the same version as other go.mod in the repo

Aditya-Gollapudi · 2021-08-11T17:58:41Z

https://github.com/C2FO/vfs

@anuraaga Is VFS approved by security - I didn't spend a bunch of time thinking about it as I was under the impression that it would be rejected by AppSec out of hand

Additionally, the goal of the config source interface is for each of the cloud providers to eventually add their own config source

github-actions · 2021-08-31T05:15:17Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

anuraaga · 2021-08-31T06:03:52Z

@Aditya-Gollapudi Sorry for missing the comment from 20 days ago just saw it now. In principle, basically nothing is approved by AWS security as they check at a macro level - even if using the AWS SDK, the usage of the AWS SDK needs to get checked, etc. On the flip side, if using a different library like vfs, then the usage of that library would be checked in a similar way. Since VFS itself just wraps the AWS SDK, this will be straightforward.

Also, this is the OpenTelemetry Collector - what's important is creating a component that makes sense for this codebase. If it's possible to make a generic mechanism for configuration across cloud providers, then that is a much better fit for this codebase than another mechanism that duplicates logic, with greater maintenance cost. If we think such a mechanism is simple, secure, and performant, than internal security review can be made to happen.

Aditya-Gollapudi · 2021-08-31T12:59:10Z

@anuraaga Thanks for getting back - I believe that AppSec (perhaps AWS security is different) did approve the usage of the SDK in the way I did above - I didn't know that it was possible to get VFS approved. If that is the case I agree it makes some sense to use it as it more generic.

However, I am no longer at Amazon for the moment so I'm not actively working on this. I believe Tianduo Hao is continuing my work (for some reason I can't seem to @skyduo)

github-actions · 2021-09-08T05:14:57Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions · 2021-09-23T05:15:37Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions · 2021-10-07T05:15:24Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

tigrannajaryan · 2021-10-12T18:27:29Z

@bogdandrutu @Aneurysm9 and I discussed how the Collector configuration can be extended to support the remote configuration needs. We do not think that the current ConfigSource approach fully covers all the uses cases and need to be extended somewhat. Please see the proposal here open-telemetry/opentelemetry-collector#4190
Until we agree on this please hold on merging this PR.

github-actions · 2021-10-20T05:15:34Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions · 2021-11-13T05:15:26Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions · 2021-12-03T05:15:44Z

Closed as inactive. Feel free to reopen if this PR is still being worked on.

Signed-off-by: Bogdan Drutu <[email protected]>

Aditya-Gollapudi and others added 15 commits June 21, 2021 09:54

Reintroduced changes builds and runs

aaf25a0

Added Dockerfile

1e67b63

added some silly changes and added containers to the specialized section

0b40587

Merge pull request #41 from Aditya-Gollapudi/aditya-test-old

3d83998

Aditya test old

Removed commented out code and reset names to match ECS Insights

57904fe

Changed code back to building from lib

b3a4177

Delete retest_config.yaml

c7de1d2

Removed a local testing file

Changed the name of the flag to be more descriptive and added some co…

01233d8

…mmenting

Changed the error logging sop that it propogated up rather than simpl…

e96b0a8

…y fatally logging

changed variable names of pod_template_hash and isPod

ec9d661

readded container metric specfic values

9177663

Removed some stray testing code

18b1e0e

I had a typo

8105312

Moving the AWS S3 Config Source PR to contrib

ecc66b7

Merge branch 'main' of https://github.com/Aditya-Gollapudi/openteleme…

5299e54

…try-collector-contrib into main

Aditya-Gollapudi requested review from anuraaga and kbrockhoff as code owners August 9, 2021 23:04

Aditya-Gollapudi requested a review from a team August 9, 2021 23:04

github-actions bot assigned jrcamp Aug 10, 2021

Aditya-Gollapudi added 3 commits August 10, 2021 09:49

added makefiles as it has been moved from collector to contrib

dd942b6

fixed a misclick from a rebase

34d9b1a

Removed the tests because I can't currently make the bucket public

83f6d5c

anuraaga reviewed Aug 11, 2021

View reviewed changes

bogdandrutu reviewed Aug 11, 2021

View reviewed changes

Aditya-Gollapudi added 2 commits August 11, 2021 11:04

Updated the go.mod file

e36a3a9

made second update to go.mod

669afa1

github-actions bot added the Stale label Aug 31, 2021

bogdandrutu removed the Stale label Aug 31, 2021

github-actions bot added the Stale label Sep 8, 2021

bogdandrutu removed the Stale label Sep 13, 2021

github-actions bot added the Stale label Sep 23, 2021

bogdandrutu removed the Stale label Sep 29, 2021

github-actions bot added the Stale label Oct 7, 2021

github-actions bot removed the Stale label Oct 13, 2021

github-actions bot added the Stale label Oct 20, 2021

bogdandrutu removed the Stale label Oct 29, 2021

github-actions bot added the Stale label Nov 13, 2021

github-actions bot closed this Dec 3, 2021

hex1848 pushed a commit to hex1848/opentelemetry-collector-contrib that referenced this pull request Jun 2, 2022

Add tests for file config map provider (open-telemetry#4575)

1941cc1

Signed-off-by: Bogdan Drutu <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Creating AWS S3 Config Source #4575

Creating AWS S3 Config Source #4575

Aditya-Gollapudi commented Aug 9, 2021

bogdandrutu commented Aug 10, 2021

anuraaga left a comment

anuraaga Aug 11, 2021

Aditya-Gollapudi Aug 11, 2021

bogdandrutu Aug 11, 2021

Aditya-Gollapudi commented Aug 11, 2021

github-actions bot commented Aug 31, 2021

anuraaga commented Aug 31, 2021 •

edited

Loading

Aditya-Gollapudi commented Aug 31, 2021

github-actions bot commented Sep 8, 2021

github-actions bot commented Sep 23, 2021

github-actions bot commented Oct 7, 2021

tigrannajaryan commented Oct 12, 2021

github-actions bot commented Oct 20, 2021

github-actions bot commented Nov 13, 2021

github-actions bot commented Dec 3, 2021

Creating AWS S3 Config Source #4575

Creating AWS S3 Config Source #4575

Conversation

Aditya-Gollapudi commented Aug 9, 2021

bogdandrutu commented Aug 10, 2021

anuraaga left a comment

Choose a reason for hiding this comment

anuraaga Aug 11, 2021

Choose a reason for hiding this comment

Aditya-Gollapudi Aug 11, 2021

Choose a reason for hiding this comment

bogdandrutu Aug 11, 2021

Choose a reason for hiding this comment

Aditya-Gollapudi commented Aug 11, 2021

github-actions bot commented Aug 31, 2021

anuraaga commented Aug 31, 2021 • edited Loading

Aditya-Gollapudi commented Aug 31, 2021

github-actions bot commented Sep 8, 2021

github-actions bot commented Sep 23, 2021

github-actions bot commented Oct 7, 2021

tigrannajaryan commented Oct 12, 2021

github-actions bot commented Oct 20, 2021

github-actions bot commented Nov 13, 2021

github-actions bot commented Dec 3, 2021

anuraaga commented Aug 31, 2021 •

edited

Loading