Skip to content

Conversation

@herin049
Copy link
Contributor

Fixes: #2068

@herin049 herin049 requested a review from a team as a code owner December 11, 2025 04:52
@serkan-ozal
Copy link
Contributor

While removing explicit AwsLambdaInstrumentor().instrument() seems OK to me, I am not fully sure on

  • might there be a side effect? did you try the changes here with a custom layer?
  • why it added before? was AwsLambdaInstrumentor not part of default list of auto instrumented packages before?

@herin049
Copy link
Contributor Author

While removing explicit AwsLambdaInstrumentor().instrument() seems OK to me, I am not fully sure on

* might there be a side effect? did you try the changes here with a custom layer?

* why it added before? was `AwsLambdaInstrumentor` not part of default list of auto instrumented packages before?

@serkan-ozal Looks like the reasoning was given here in a comment which I accidentally missed:

Usage
-----
Patch the reserved `_HANDLER` Lambda environment variable to point to this
file's `otel_wrapper.lambda_handler` property. Do this having saved the original
`_HANDLER` in the `ORIG_HANDLER` environment variable. Doing this makes it so
that **on import of this file, the handler is instrumented**.

Instrumenting any earlier will cause the instrumentation to be lost because the
AWS Service uses `imp.load_module` to import the handler which RELOADS the
module. This is why AwsLambdaInstrumentor cannot be instrumented with the
`opentelemetry-instrument` script.

Looks like this was added by @NathanielRN

However, I don't think the reasoning here is quite correct. Even IF lambda does reload the module being pointed to by _HANDLER, this will not matter because the original lambda handler pointed to by ORIG_HANDLER lives in a different module and thus, will NOT be reloaded if the lambda runtime reloads the module pointed to by _HANLDER (which is the otel_wrapper module)

(see https://docs.python.org/3/library/importlib.html#importlib.reload)

Even the original approach doesn't accomplish the goal of instrumenting the lambda handler inside the wrapper script because the AWS Lambda instrumentation library is not excluded from being used by auto instrumentation. As a result, the lambda instrumentation library is called before the wrapper script is run and any subsequent attempts to instrument will be a no-op due to the short-circuiting logic inside the Python instrumentation library:

    def instrument(self, **kwargs: Any):
        """Instrument the library

        This method will be called without any optional arguments by the
        ``opentelemetry-instrument`` command.

        This means that calling this method directly without passing any
        optional values should do the very same thing that the
        ``opentelemetry-instrument`` command does.
        """

        if self._is_instrumented_by_opentelemetry:
            _LOG.warning("Attempting to instrument while already instrumented")
            return None

I've tested these changes by building the auto instrumentation layer exactly how it is in this PR with Python 3.14, if you'd like, I can verify that this works on the older lambda runtimes. This also makes me think that we should at some point add an integration test suite that runs in AWS against all supported lambda runtimes - at least before releasing new layer versions. Let me know what you think, we can discuss details further in one of the FaaS SIG meetings. This is something I would be very happy to lead development on.

@serkan-ozal
Copy link
Contributor

serkan-ozal commented Dec 13, 2025

@herin049 I would test with older (but currently available) Python Lambda runtimes too. AWS Lambda Python runtime clients (which includes bootstrap.py) might have different implementations (we have faced some cases in Nodejs) and AwsLambdaInstrumentor().instrument() call might have been added for some weird trick/case in one of the older Lambda Python runtimes.

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.

Python: Attempt to instrument while already instrument warning

2 participants