Add AWS resource detectors to extension package

[NathanielRN]

Description

This PR adds Resource Detectors to automatically populate attributes under the resource namespace of every span. The included Resource Detectors are for the following AWS Service:

• AWS Lambda
• AWS Beanstalk
• AWS EC2
• AWS ECS
• AWS EKS

This is similar to packages already available on OpenTelemetry JavaScript and OpenTelemetry Java.

Type of change

Please delete options that are not relevant.

• New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

• All ResourceDetectors have unit tests to verify file reading/API conformance
• For all ResourceDetectors, I was able to run OTel Python on these services and verify that the spans are populated with the right attributes.
• Beanstalk + EKS had additional non-obvious steps so for those I added links in the docstring!

Does This PR Require a Core Repo Change?

• No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

• Followed the style guidelines of this project
• Changelogs have been updated
• Unit tests have been added
• Documentation has been updated

[NathanielRN]

Blocked on #585, converting to draft.

[srprash]

LGTM! Thanks for doing this.

[lzchen]

Nit: opentelemetry.sdk.extension.aws.resources plural to match opentelemetry.sdk.resources? Not a big deal though.

[NathanielRN]

Oh I didn't even notice that, actually I did resource because that is the folder it is in on the specifications

Also in java we put it under resource

And yet JavaScript, Go and PHP all put it under a folder called detectors/ 😅

I guess I'll defer to the SIG here, as GCP and other vendors will probably add their resource Detectors soon... maybe we should even move the SDK import path? 😓 Although I imagine that will be hard after 1.0.

[lzchen]

We aren't at 1.0 yet so nows the time to decide on an import path :D. But again, not a blocker.

[NathanielRN]

Ah, I meant we should move opentelemetry.sdk.resources to opentelemetry.sdk.resource to match the spec 🙂

I actually like .resource because there is only 1 resource: namespace in the attributes. Even if you add multiple ResourceDetectors you're still setting attributes in 1 namespace...

I'll leave it for now and if we need to we'll change it before this package goes 1.0 😄. Shouldn't be a big deal to do "both" if we need to as well later!

[lzchen]

I think it'll be better to catch this specific type of exception below than to have nested general exceptions caught.

[NathanielRN]

If I understand you correctly, you think it would be better than we catch FileNotFoundError specifically? That makes sense to me! I'll update it here and in the eks.py file.

[aabmass]

LGTM

[aabmass]

Hmm it would have been nice if this was handled in the ResourceDetector super class so the implementations didn't have to duplicate it every time, but too late now I think 🤷‍♂️

[NathanielRN]

That's a really good point, do you mean something like this on opentelemetry-python-core?:

class ResourceDetector(abc.ABC):
    def __init__(self, raise_on_error=False):
        self.raise_on_error = raise_on_error

    @abc.abstractmethod
    def _detect(self) -> "Resource":
        raise NotImplementedError()

    def detect(self) -> "Resource":
        try:
            self._detect()
        except NotImplementedError as exception:
            raise exception
        # pylint: disable=broad-except
        except Exception as exception:
            if self.raise_on_error:
                raise exception
            
            logger.warning(f"{self.__class__.__name__} failed: {exception}")
            return Resource.get_empty()

I think that would definitely be worth it, maybe we can add it in a minor release for now and support (but deprecate) the existing method?

If this sounds like it makes sense I can open a small PR with this change on core! 🙂

[aabmass]

Is logging this redudant if we a re re-raising?

[NathanielRN]

Hm that's a good point, I was hoping to add class name to the exception as e_msg = f"{self.__class__.__name__} failed: {exception}", but they can see it as been obvious enough from the Traceback.

Thanks! I'll update these exceptions across the detectors 🙂