Skip to content

Conversation

@liustve
Copy link
Contributor

@liustve liustve commented May 2, 2025

Background
Supporting ADOT auto instrumentation to automatically inject SigV4 authentication headers for outgoing export log requests to the allow exporting to the AWS Logs OTLP endpoint. Users will need to configure the following environment variables in order to enable and properly run this exporter:

OTEL_EXPORTER_OTLP_LOGS_ENDPOINT=https://logs.[AWS-REGION].amazonaws.com/v1/logs; required
OTEL_EXPORTER_OTLP_LOGS_HEADERS=x-aws-log-group=[CW-LOG-GROUP-NAME],x-aws-log-stream=[CW-LOG-STREAM-NAME] required
OTEL_PYTHON_LOGGING_AUTO_INSTRUMENTATION_ENABLED=true required
OTEL_LOGS_EXPORTER=otlp required or do not set env variable
OTEL_EXPORTER_OTLP_LOGS_PROTOCOL=http/protobuf required or do not set env variable
OTEL_METRICS_EXPORTER=none

Description of changes:

  1. Add new AwsAuthSession class to inject Sigv4 headers directly into the sessions object used by the upstream exporter.
    https://github.com/srprash/aws-otel-sigv4-auth/tree/main

  2. The ADOT auto instrumentation is now configured to automatically detect if a user is exporting to CW Logs OTLP Logs endpoint by checking if the environment variable OTEL_EXPORTER_OTLP_LOGS_ENDPOINT is configured to match this url pattern: https://logs.[AWS-REGION].amazonaws.com/v1/logs

Testing:

  1. E2E test done in an empty EC2 environment without configuring .aws credentials config file or setting AWS credentials in the environment variable
  2. Manual testing was done by configuring the above environment variables and setting up the sample app locally with ADOT auto instrumentation and verified the spans in CW Logs.
  3. The sample app was run and rerun 30 times and confirmed no issues with exporting the logs to the endpoint
  4. Unit tests were added to verify functionality of OtlpAwsLogsExporter

Further testing will be done with the Release tests.

Example of a log exported using this exporter:

{
    "resource": {
        "attributes": {
            "aws.local.service": "test",
            "service.name": "test",
            "cloud.region": "us-west-2",
            "host.type": "c5.4xlarge",
            "cloud.availability_zone": "us-west-2c",
            "telemetry.sdk.name": "opentelemetry",
            "telemetry.sdk.language": "python",
            "cloud.provider": "aws",
            "cloud.account.id": "571600841604",
            "telemetry.sdk.version": "1.27.0",
            "host.name": "ip-172-31-7-29.us-west-2.compute.internal",
            "cloud.platform": "aws_ec2",
            "host.id": "i-0b04d6affbae7d629",
            "telemetry.auto.version": "0.9.0.dev0-aws"
        }
    },
    "scope": {
        "name": "opentelemetry.sdk._logs._internal"
    },
    "timeUnixNano": 1746221090210187520,
    "observedTimeUnixNano": 1746221090210645309,
    "severityNumber": 9,
    "severityText": "INFO",
    "body": "\u001b[31m\u001b[1mWARNING: This is a development server. Do not use it in a production deployment. Use a production WSGI server instead.\u001b[0m\n * Running on http://127.0.0.1:8082",
    "attributes": {
        "code.filepath": "/home/ec2-user/aws-otel-python-instrumentation/auto_instrumentation/lib64/python3.11/site-packages/werkzeug/_internal.py",
        "otelTraceSampled": false,
        "code.function": "_log",
        "code.lineno": 97,
        "otelTraceID": "0",
        "otelSpanID": "0",
        "otelServiceName": "test"
    },
    "traceId": "",
    "spanId": ""
}

@liustve liustve requested a review from a team as a code owner May 2, 2025 22:12
@liustve liustve marked this pull request as draft May 2, 2025 22:12
@yiyuan-he
Copy link
Contributor

Thanks Steve - Code structure looks clean overall with good separation of concerns. Did some E2E testing as well and can confirm functionality.

My only concern is the approach to use multiple inheritance:

  1. Python's method resolution order can be unintuitive when methods are overriden in multiple parent classes.
  2. The code manually calls both parent __init__ methods in a specific order. This is fragile and could break if either parent class implementation changes.
  3. When upstream makes changes to parent class, the multiple inheritance can make tracking changes more difficult.

Perhaps we could consider another approach where we simplify by just extracting the common SigV4 auth into a utility function or class.

For example:

# For the span exporter
class OTLPAwsSpanExporter(OTLPSpanExporter):
    def _export(self, serialized_data: bytes):
        from amazon.opentelemetry.distro.exporter.otlp.aws.common.sigv4_auth import apply_sigv4_auth
        apply_sigv4_auth(self._endpoint, self._session, "xray", serialized_data)
        return super()._export(serialized_data)

# For the logs exporter
class OTLPAwsLogExporter(OTLPLogExporter):
    def _export(self, serialized_data: bytes):
        from amazon.opentelemetry.distro.exporter.otlp.aws.common.sigv4_auth import apply_sigv4_auth
        apply_sigv4_auth(self._endpoint, self._session, "logs", serialized_data)
        return super()._export(serialized_data)

@liustve liustve marked this pull request as ready for review May 5, 2025 22:03
Copy link
Contributor

@srprash srprash left a comment

Choose a reason for hiding this comment

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

Also, you would want to update the PR description since it is no longer accurate. :)

@liustve liustve merged commit cb3a1b0 into aws-observability:main May 9, 2025
12 checks passed
@liustve liustve deleted the sigv4_logs branch May 9, 2025 22:17
liustve added a commit that referenced this pull request May 12, 2025
*Background:*

#358
The above PR got rid of OtlpAwsSpanExporter and OtlpAwsLogExporter as to
use the default upstream exporters as that was a cleaner approach.

However, we need the OtlpAwsSpanExporter and OtlpAwsLogExporter classes
to support later requirements for Gen AI as we need to override the
export method.

*Description of changes:*
This PR reintroduces the OtlpAwsSpanExporter and OtlpAwsLogExporter
classes to support later requirements for Gen AI as we need to override
the export method.

This change does not introduce anything new and the Sigv4 span + logs
exporter still work as intended:

Logs:
```
{
    "resource": {
        "attributes": {
            "aws.local.service": "test",
            "service.name": "test",
            "cloud.region": "us-west-2",
            "host.type": "c5.4xlarge",
            "cloud.availability_zone": "us-west-2c",
            "telemetry.sdk.name": "opentelemetry",
            "telemetry.sdk.language": "python",
            "cloud.provider": "aws",
            "cloud.account.id": "571600841604",
            "telemetry.sdk.version": "1.27.0",
            "host.name": "ip-172-31-7-29.us-west-2.compute.internal",
            "cloud.platform": "aws_ec2",
            "host.id": "i-0b04d6affbae7d629",
            "telemetry.auto.version": "0.9.0.dev0-aws"
        }
    },
    "scope": {
        "name": "opentelemetry.sdk._logs._internal"
    },
    "timeUnixNano": 1747074906326769664,
    "observedTimeUnixNano": 1747074906326822815,
    "severityNumber": 9,
    "severityText": "INFO",
    "body": "127.0.0.1 - - [12/May/2025 18:35:06] \"GET /server_request HTTP/1.1\" 200 -",
    "attributes": {
        "code.filepath": "/home/ec2-user/.local/lib/python3.9/site-packages/werkzeug/_internal.py",
        "otelTraceSampled": false,
        "code.function": "_log",
        "code.lineno": 97,
        "otelTraceID": "0",
        "otelSpanID": "0",
        "otelServiceName": "test"
    },
    "traceId": "",
    "spanId": ""
}
```

Spans:

```



{
    "resource": {
        "attributes": {
            "aws.local.service": "test",
            "service.name": "test",
            "cloud.region": "us-west-2",
            "host.type": "c5.4xlarge",
            "cloud.availability_zone": "us-west-2c",
            "telemetry.sdk.name": "opentelemetry",
            "telemetry.sdk.language": "python",
            "cloud.provider": "aws",
            "cloud.account.id": "571600841604",
            "telemetry.sdk.version": "1.27.0",
            "host.name": "ip-172-31-7-29.us-west-2.compute.internal",
            "cloud.platform": "aws_ec2",
            "host.id": "i-0b04d6affbae7d629",
            "telemetry.auto.version": "0.9.0.dev0-aws"
        }
    },
    "scope": {
        "name": "opentelemetry.instrumentation.flask",
        "version": "0.48b0"
    },
    "traceId": "68223f2d375733237e24512171012437",
    "spanId": "34dd5a89d7a21fdf",
    "flags": 256,
    "name": "GET /",
    "kind": "SERVER",
    "startTimeUnixNano": 1747074861988123056,
    "endTimeUnixNano": 1747074861988911516,
    "durationNano": 788460,
    "attributes": {
        "net.host.name": "localhost:8082",
        "aws.local.service": "test",
        "net.peer.port": 57356,
        "telemetry.extended": "true",
        "http.target": "/",
        "http.flavor": "1.1",
        "net.peer.ip": "127.0.0.1",
        "http.host": "localhost:8082",
        "aws.local.environment": "ec2:default",
        "http.status_code": 404,
        "aws.local.operation": "GET /",
        "aws.span.kind": "LOCAL_ROOT",
        "http.server_name": "127.0.0.1",
        "http.user_agent": "curl/8.5.0",
        "net.host.port": 8082,
        "PlatformType": "AWS::EC2",
        "http.method": "GET",
        "http.response.status_code": 404,
        "http.scheme": "http"
    },
    "status": {
        "code": "UNSET"
    }
}

```

By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants