remove unnecessary call to AwsLambdaInstrumentor().instrument() in wrapper script
Fixes: #2068
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
AwsLambdaInstrumentornot part of default list of auto instrumented packages before?
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.
@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.
@serkan-ozal I was able to test and manually verify that the changes work with Python 3.9, 3.11, 3.12, 3.13 and 3.14.
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.
Talked about this in the SIG meeting with @tylerbenson today as well. I have an issue open here to track this effort as well: https://github.com/open-telemetry/opentelemetry-lambda/issues/2079