Skip to content

Updated EC2 detector to use v2 aws sdk #6878

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,14 @@ migration](https://github.com/open-telemetry/semantic-conventions/blob/main/docs
The `code.namespace` attribute is no longer added. (#6870)
- The `code.function` attribute emitted by `go.opentelemetry.io/contrib/bridges/otelzap` now stores the package path-qualified function name instead of just the function name.
The `code.namespace` attribute is no longer added. (#6870)
- Updated AWS EC2 detector to use v2 version of go AWS SDK import `go.opentelemetry.io/contrib/detectors/aws/ec2`.
This release still has the AWS SDK v1 as a dependency as we deprecate the public `Client` struct. (#6878)

### Deprecated

- Deprecate `WithAttributeSetter`, `AttributeSetter`, `DefaultAttributeSetter`, `DynamoDBAttributeSetter`, `SNSAttributeSetter` in favor of `WithAttributeBuilder`, `AttributeBuilder`, `DefaultAttributeBuilder`, `DynamoDBAttributeBuilder`, `SNSAttributeBuilder` in `go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws` (#6543)
- Deprecate `go.opentelemetry.io/contrib/config` module in favor of `go.opentelemetry.io/contrib/otelconf`. This is the last release of this module. (#6796)
- Deprecate public `Client` in `go.opentelemetry.io/contrib/detectors/aws/ec2`. (#6878)

### Fixed

Expand Down
71 changes: 45 additions & 26 deletions detectors/aws/ec2/ec2.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,21 @@ import (
"context"
"errors"
"fmt"
"io"
"net/http"

"github.com/aws/aws-sdk-go/aws/awserr"
awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http"
awsconfig "github.com/aws/aws-sdk-go-v2/config"
"github.com/aws/aws-sdk-go-v2/feature/ec2/imds"
"github.com/aws/aws-sdk-go/aws/ec2metadata"
"github.com/aws/aws-sdk-go/aws/session"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/sdk/resource"
semconv "go.opentelemetry.io/otel/semconv/v1.26.0"
)

type config struct {
c Client
c client
}

// newConfig returns an appropriately configured config.
Expand All @@ -44,28 +46,39 @@ func (fn optionFunc) apply(c *config) {
}

// WithClient sets the ec2metadata client in config.
func WithClient(t Client) Option {
func WithClient(t client) Option {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this is going to be problematic.

The function signature here cannot change, that will be a breaking change

  1. This could be deprecated as well. It needs to be verified that there is never a need for a client to be set here. Likely this is not the case if a user is trying to stub out functionality for testing external to this package.
  2. The Client interface could be extended and a type assertion can be made that will check if the passed client implements the prior functionality. This will mean that backwards compatible support for the existing API needs to be possible (even with the upgrade).
  3. A ec2/v2 package is created to support the AWS v2 API and this package is deprecated
  4. Maybe some other options ... (?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yeah I'm not sure if this just also should've been unexported along with Client or if there's a valid use case for a user to set their own? I see the commit that added it on the off chance you remember the rational behind it #1030 @MrAlias.

If we create a v2 package and deprecate this one, does the v2 package permanently stay as v2

Copy link
Member

@pellared pellared Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://go.dev/blog/v2-go-modules

The recommended strategy is to develop v2+ modules in a directory named after the major version suffix.

This allows also nicely deprecating the existing v1 module.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plan from SIG meeting is to create a v2 of this module.

return optionFunc(func(c *config) {
c.c = t
})
}

func (cfg *config) getClient() Client {
func (cfg *config) getClient() client {
return cfg.c
}

// resource detector collects resource information from EC2 environment.
type resourceDetector struct {
c Client
c client
}

// Client implements methods to capture EC2 environment metadata information.
//
// Deprecated: Unnecessary public client. This will be removed in a future release.
type Client interface {
Available() bool
GetInstanceIdentityDocument() (ec2metadata.EC2InstanceIdentityDocument, error)
GetMetadata(p string) (string, error)
}

// client implements methods to capture EC2 environment metadata information.
type client interface {
GetInstanceIdentityDocument(ctx context.Context, params *imds.GetInstanceIdentityDocumentInput, optFns ...func(*imds.Options)) (*imds.GetInstanceIdentityDocumentOutput, error)
GetMetadata(ctx context.Context, params *imds.GetMetadataInput, optFns ...func(*imds.Options)) (*imds.GetMetadataOutput, error)
}

// compile time assertion that imds.Client implements client.
var _ client = (*imds.Client)(nil)

// compile time assertion that resourceDetector implements the resource.Detector interface.
var _ resource.Detector = (*resourceDetector)(nil)

Expand All @@ -77,18 +90,15 @@ func NewResourceDetector(opts ...Option) resource.Detector {

// Detect detects associated resources when running in AWS environment.
func (detector *resourceDetector) Detect(ctx context.Context) (*resource.Resource, error) {
client, err := detector.client()
client, err := detector.client(ctx)
if err != nil {
return nil, err
}

if !client.Available() {
return nil, nil
}

doc, err := client.GetInstanceIdentityDocument()
// Available method removed in aws-sdk-go-v2, return nil if client returns error
doc, err := client.GetInstanceIdentityDocument(ctx, nil)
if err != nil {
return nil, err
return nil, nil
}

attributes := []attribute.KeyValue{
Expand All @@ -103,7 +113,7 @@ func (detector *resourceDetector) Detect(ctx context.Context) (*resource.Resourc
}

m := &metadata{client: client}
m.add(semconv.HostNameKey, "hostname")
m.add(ctx, semconv.HostNameKey, "hostname")

attributes = append(attributes, m.attributes...)

Expand All @@ -114,42 +124,51 @@ func (detector *resourceDetector) Detect(ctx context.Context) (*resource.Resourc
return resource.NewWithAttributes(semconv.SchemaURL, attributes...), err
}

func (detector *resourceDetector) client() (Client, error) {
func (detector *resourceDetector) client(ctx context.Context) (client, error) {
if detector.c != nil {
return detector.c, nil
}

s, err := session.NewSession()
cfg, err := awsconfig.LoadDefaultConfig(ctx)
if err != nil {
return nil, err
}

return ec2metadata.New(s), nil
return imds.NewFromConfig(cfg), nil
}

type metadata struct {
client Client
client client
errs []error
attributes []attribute.KeyValue
}

func (m *metadata) add(k attribute.Key, n string) {
v, err := m.client.GetMetadata(n)
if err == nil {
m.attributes = append(m.attributes, k.String(v))
func (m *metadata) add(ctx context.Context, k attribute.Key, n string) {
metadataInput := &imds.GetMetadataInput{Path: n}
md, err := m.client.GetMetadata(ctx, metadataInput)
if err != nil {
m.recordError(n, err)
return
}
data, err := io.ReadAll(md.Content)
if err != nil {
m.recordError(n, err)
return
}
m.attributes = append(m.attributes, k.String(string(data)))
}

var rf awserr.RequestFailure
func (m *metadata) recordError(path string, err error) {
var rf *awshttp.ResponseError
ok := errors.As(err, &rf)
if !ok {
m.errs = append(m.errs, fmt.Errorf("%q: %w", n, err))
m.errs = append(m.errs, fmt.Errorf("%q: %w", path, err))
return
}

if rf.StatusCode() == http.StatusNotFound {
if rf.HTTPStatusCode() == http.StatusNotFound {
return
}

m.errs = append(m.errs, fmt.Errorf("%q: %d %s", n, rf.StatusCode(), rf.Code()))
m.errs = append(m.errs, fmt.Errorf("%q: %d %s", path, rf.HTTPStatusCode(), rf.Error()))
}
101 changes: 69 additions & 32 deletions detectors/aws/ec2/ec2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,19 @@
package ec2

import (
"bytes"
"context"
"errors"
"io"
"net/http"
"testing"
"time"

"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/aws/ec2metadata"
awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http"
"github.com/aws/aws-sdk-go-v2/feature/ec2/imds"
"github.com/aws/smithy-go"
smithyhttp "github.com/aws/smithy-go/transport/http"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand All @@ -22,7 +27,7 @@ import (

func TestAWS_Detect(t *testing.T) {
type fields struct {
Client Client
Client client
}

type want struct {
Expand All @@ -31,9 +36,9 @@ func TestAWS_Detect(t *testing.T) {
Resource *resource.Resource
}

usWestInst := func() (ec2metadata.EC2InstanceIdentityDocument, error) {
usWestInst := func() (*imds.GetInstanceIdentityDocumentOutput, error) {
// Example from https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-identity-documents.html
doc := ec2metadata.EC2InstanceIdentityDocument{
doc := imds.InstanceIdentityDocument{
MarketplaceProductCodes: []string{"1abc2defghijklm3nopqrs4tu"},
AvailabilityZone: "us-west-2b",
PrivateIP: "10.158.112.84",
Expand All @@ -47,7 +52,9 @@ func TestAWS_Detect(t *testing.T) {
Architecture: "x86_64",
}

return doc, nil
return &imds.GetInstanceIdentityDocumentOutput{
InstanceIdentityDocument: doc,
}, nil
}

usWestIDLabels := []attribute.KeyValue{
Expand All @@ -65,44 +72,51 @@ func TestAWS_Detect(t *testing.T) {
Fields fields
Want want
}{
"Unavailable": {
Fields: fields{Client: &clientMock{}},
},
"Instance ID Error": {
Fields: fields{
Client: &clientMock{available: true, idDoc: func() (ec2metadata.EC2InstanceIdentityDocument, error) {
return ec2metadata.EC2InstanceIdentityDocument{}, errors.New("id not available")
Client: &clientMock{idDoc: func() (*imds.GetInstanceIdentityDocumentOutput, error) {
return &imds.GetInstanceIdentityDocumentOutput{}, errors.New("id not available")
}},
},
Want: want{Error: "id not available"},
},
"Hostname Not Found": {
Fields: fields{
Client: &clientMock{available: true, idDoc: usWestInst, metadata: map[string]meta{}},
Client: &clientMock{idDoc: usWestInst, metadata: map[string]meta{}},
},
Want: want{Resource: resource.NewWithAttributes(semconv.SchemaURL, usWestIDLabels...)},
},
"Hostname Response Error": {
Fields: fields{
Client: &clientMock{
available: true,
idDoc: usWestInst,
idDoc: usWestInst,
metadata: map[string]meta{
"hostname": {err: awserr.NewRequestFailure(awserr.New("EC2MetadataError", "failed to make EC2Metadata request", errors.New("response error")), http.StatusInternalServerError, "test-request")},
"hostname": {
err: &awshttp.ResponseError{
ResponseError: &smithyhttp.ResponseError{
Err: errors.New("EC2MetadataError"),
Response: &smithyhttp.Response{
Response: &http.Response{
StatusCode: 500,
},
},
},
RequestID: "test-request",
},
},
},
},
},
Want: want{
Error: `partial resource: ["hostname": 500 EC2MetadataError]`,
Error: `partial resource: ["hostname": 500 https response error StatusCode: 500, RequestID: test-request, EC2MetadataError]`,
Partial: true,
Resource: resource.NewWithAttributes(semconv.SchemaURL, usWestIDLabels...),
},
},
"Hostname General Error": {
Fields: fields{
Client: &clientMock{
available: true,
idDoc: usWestInst,
idDoc: usWestInst,
metadata: map[string]meta{
"hostname": {err: errors.New("unknown error")},
},
Expand All @@ -117,8 +131,7 @@ func TestAWS_Detect(t *testing.T) {
"All Available": {
Fields: fields{
Client: &clientMock{
available: true,
idDoc: usWestInst,
idDoc: usWestInst,
metadata: map[string]meta{
"hostname": {value: "ip-12-34-56-78.us-west-2.compute.internal"},
},
Expand Down Expand Up @@ -163,29 +176,53 @@ func TestAWS_Detect(t *testing.T) {
}

type clientMock struct {
available bool
idDoc func() (ec2metadata.EC2InstanceIdentityDocument, error)
metadata map[string]meta
idDoc func() (*imds.GetInstanceIdentityDocumentOutput, error)
metadata map[string]meta
}

type meta struct {
err error
value string
}

func (c *clientMock) Available() bool {
return c.available
}

func (c *clientMock) GetInstanceIdentityDocument() (ec2metadata.EC2InstanceIdentityDocument, error) {
func (c *clientMock) GetInstanceIdentityDocument(ctx context.Context, params *imds.GetInstanceIdentityDocumentInput, optFns ...func(*imds.Options)) (*imds.GetInstanceIdentityDocumentOutput, error) {
return c.idDoc()
}

func (c *clientMock) GetMetadata(p string) (string, error) {
v, ok := c.metadata[p]
func (c *clientMock) GetMetadata(ctx context.Context, params *imds.GetMetadataInput, optFns ...func(*imds.Options)) (*imds.GetMetadataOutput, error) {
v, ok := c.metadata[params.Path]
if !ok {
return "", awserr.NewRequestFailure(awserr.New("EC2MetadataError", "failed to make EC2Metadata request", errors.New("response error")), http.StatusNotFound, "test-request")
return nil, createEC2MetadataError()
}

return v.value, v.err
if v.err != nil {
return nil, v.err
}

return &imds.GetMetadataOutput{
Content: io.NopCloser(bytes.NewReader([]byte(v.value))),
}, nil
}

func createEC2MetadataError() error {
smithyResp := &smithyhttp.Response{
Response: &http.Response{
StatusCode: http.StatusNotFound,
Status: "404 Not Found",
},
}

smithyRespErr := &smithyhttp.ResponseError{
Response: smithyResp,
Err: &smithy.GenericAPIError{
Code: "EC2MetadataError",
Message: "failed to make EC2Metadata request",
Fault: smithy.FaultClient,
},
}

return &awshttp.ResponseError{
ResponseError: smithyRespErr,
RequestID: "test-request",
}
}
13 changes: 13 additions & 0 deletions detectors/aws/ec2/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,25 @@ go 1.22.0

require (
github.com/aws/aws-sdk-go v1.55.6

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we plan on mitigating this https://nvd.nist.gov/vuln/detail/CVE-2020-8911 and https://nvd.nist.gov/vuln/detail/CVE-2020-8912 if we are still pulling in this dependency?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plan from SIG meeting is to create a v2 of this module.

This should solve it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This package will be deprecated as is, a v2 will be created without this dependency

github.com/aws/aws-sdk-go-v2 v1.36.3
github.com/aws/aws-sdk-go-v2/config v1.29.8
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.30
github.com/aws/smithy-go v1.22.2
github.com/stretchr/testify v1.10.0
go.opentelemetry.io/otel v1.34.0
go.opentelemetry.io/otel/sdk v1.34.0
)

require (
github.com/aws/aws-sdk-go-v2/credentials v1.17.61 // indirect
github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.34 // indirect
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.34 // indirect
github.com/aws/aws-sdk-go-v2/internal/ini v1.8.3 // indirect
github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.3 // indirect
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.15 // indirect
github.com/aws/aws-sdk-go-v2/service/sso v1.25.0 // indirect
github.com/aws/aws-sdk-go-v2/service/ssooidc v1.29.0 // indirect
github.com/aws/aws-sdk-go-v2/service/sts v1.33.16 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/go-logr/logr v1.4.2 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
Expand Down
Loading