Skip to content

Update Python Sample Apps for Upstream 1.3 #110

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

Merged
merged 3 commits into from
Jul 6, 2021
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
)

# Enable instrumentation
AwsLambdaInstrumentor().instrument()
AwsLambdaInstrumentor().instrument(skip_dep_check=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use flag skip_dep_check to remove these code ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm I'm not sure what we would replace it with. The _load_distros, _load_instrumentors and _load_configurators all initialize the classes they find. Normally, its important to not skip_dep_check because the classes _load_instrumentors loads requires that the dependencies are installed. Attempting to instrument without the dependencies would cause an import error.

I think it might be possible to use the upstream opentelemetry-instrumentation package as it is today instead of doing this code ourselves, but we don't need that in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's merge the code first. I feel if is_recording() is essential why it is not integrated inside of start_span. I can raise issue in python repo later.


# Lambda function
def lambda_handler(event, context):
Expand All @@ -47,36 +47,47 @@ def lambda_handler(event, context):
import logging
import os
from importlib import import_module

from typing import Collection
from wrapt import wrap_function_wrapper

# TODO: aws propagator
from opentelemetry.sdk.extension.aws.trace.propagation.aws_xray_format import (
AwsXRayFormat,
)
from opentelemetry.instrumentation.aws_lambda.package import _instruments
from opentelemetry.instrumentation.aws_lambda.version import __version__
from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
from opentelemetry.instrumentation.utils import unwrap
from opentelemetry.semconv.trace import SpanAttributes
from opentelemetry.trace import SpanKind, get_tracer, get_tracer_provider

logger = logging.getLogger(__name__)


class AwsLambdaInstrumentor(BaseInstrumentor):
def _instrument(self, **kwargs):
self._tracer = get_tracer(__name__, __version__, kwargs.get("tracer_provider"))
def instrumentation_dependencies(self) -> Collection[str]:
return _instruments

self._tracer_provider = get_tracer_provider()
def _instrument(self, **kwargs):
"""Instruments Lambda Handlers on AWS Lambda

Args:
**kwargs: Optional arguments
``tracer_provider``: a TracerProvider, defaults to global
"""
tracer = get_tracer(
__name__, __version__, kwargs.get("tracer_provider")
)

lambda_handler = os.environ.get("ORIG_HANDLER", os.environ.get("_HANDLER"))
lambda_handler = os.environ.get(
"ORIG_HANDLER", os.environ.get("_HANDLER")
)
wrapped_names = lambda_handler.rsplit(".", 1)
self._wrapped_module_name = wrapped_names[0]
self._wrapped_function_name = wrapped_names[1]

wrap_function_wrapper(
self._wrapped_module_name,
self._wrapped_function_name,
self._functionPatch,
_instrument(
tracer, self._wrapped_module_name, self._wrapped_function_name
)

def _uninstrument(self, **kwargs):
Expand All @@ -85,35 +96,50 @@ def _uninstrument(self, **kwargs):
self._wrapped_function_name,
)

def _functionPatch(self, original_func, instance, args, kwargs):
lambda_context = args[1]
ctx_aws_request_id = lambda_context.aws_request_id
ctx_invoked_function_arn = lambda_context.invoked_function_arn
orig_handler = os.environ.get("ORIG_HANDLER", os.environ.get("_HANDLER"))

def _instrument(tracer, wrapped_module_name, wrapped_function_name):
def _instrumented_lambda_handler_call(call_wrapped, instance, args, kwargs):
orig_handler_name = ".".join(
[wrapped_module_name, wrapped_function_name]
)

# TODO: enable propagate from AWS by env variable
xray_trace_id = os.environ.get("_X_AMZN_TRACE_ID", "")

lambda_name = os.environ.get("AWS_LAMBDA_FUNCTION_NAME")
function_version = os.environ.get("AWS_LAMBDA_FUNCTION_VERSION")

propagator = AwsXRayFormat()
parent_context = propagator.extract({"X-Amzn-Trace-Id": xray_trace_id})

with self._tracer.start_as_current_span(
name=orig_handler, context=parent_context, kind=SpanKind.SERVER
with tracer.start_as_current_span(
name=orig_handler_name, context=parent_context, kind=SpanKind.SERVER
) as span:
# Refer: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/faas.md#example
span.set_attribute("faas.execution", ctx_aws_request_id)
span.set_attribute("faas.id", ctx_invoked_function_arn)

# TODO: fix in Collector because they belong resource attrubutes
span.set_attribute("faas.name", lambda_name)
span.set_attribute("faas.version", function_version)

result = original_func(*args, **kwargs)
if span.is_recording():
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure it's necessary. When start the span and sampled, is_recording() is always true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good question! Initially I didn't know why we did this and I just did it because we do it for every other instrumentation as seen in open-telemetry/opentelemetry-python#1057, so I wanted AwsLambdaInstrumentor to be the same.

But now I understand that we need this because we don't know if the Span will be Sampled. It is started like you said but if the Sampler doesn't sample this span then we shouldn't record attributes.

lambda_context = args[1]
# Refer: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/faas.md#example
span.set_attribute(
SpanAttributes.FAAS_EXECUTION, lambda_context.aws_request_id
)
span.set_attribute(
"faas.id", lambda_context.invoked_function_arn
)

# TODO: fix in Collector because they belong resource attrubutes
span.set_attribute(
"faas.name", os.environ.get("AWS_LAMBDA_FUNCTION_NAME")
)
span.set_attribute(
"faas.version",
os.environ.get("AWS_LAMBDA_FUNCTION_VERSION"),
)

result = call_wrapped(*args, **kwargs)

# force_flush before function quit in case of Lambda freeze.
self._tracer_provider.force_flush()
tracer_provider = get_tracer_provider()
tracer_provider.force_flush()

return result

wrap_function_wrapper(
wrapped_module_name,
wrapped_function_name,
_instrumented_lambda_handler_call,
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Copyright The OpenTelemetry Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.


_instruments = ()
2 changes: 1 addition & 1 deletion python/src/otel/otel_sdk/otel_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class HandlerError(Exception):
_load_configurators()
_load_instrumentors()
# TODO: move to python-contrib
AwsLambdaInstrumentor().instrument()
AwsLambdaInstrumentor().instrument(skip_dep_check=True)

path = os.environ.get("ORIG_HANDLER", None)
if path is None:
Expand Down
54 changes: 27 additions & 27 deletions python/src/otel/otel_sdk/requirements-nodeps.txt
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
opentelemetry-instrumentation-aiohttp-client==0.21b0
opentelemetry-instrumentation-asgi==0.21b0
opentelemetry-instrumentation-asyncpg==0.21b0
opentelemetry-instrumentation-boto==0.21b0
opentelemetry-instrumentation-botocore==0.21b0
opentelemetry-instrumentation-celery==0.21b0
opentelemetry-instrumentation-dbapi==0.21b0
opentelemetry-instrumentation-django==0.21b0
opentelemetry-instrumentation-elasticsearch==0.21b0
opentelemetry-instrumentation-fastapi==0.21b0
opentelemetry-instrumentation-falcon==0.21b0
opentelemetry-instrumentation-flask==0.21b0
opentelemetry-instrumentation-grpc==0.21b0
opentelemetry-instrumentation-jinja2==0.21b0
opentelemetry-instrumentation-mysql==0.21b0
opentelemetry-instrumentation-psycopg2==0.21b0
opentelemetry-instrumentation-pymemcache==0.21b0
opentelemetry-instrumentation-pymongo==0.21b0
opentelemetry-instrumentation-pymysql==0.21b0
opentelemetry-instrumentation-pyramid==0.21b0
opentelemetry-instrumentation-redis==0.21b0
opentelemetry-instrumentation-requests==0.21b0
opentelemetry-instrumentation-sqlalchemy==0.21b0
opentelemetry-instrumentation-sqlite3==0.21b0
opentelemetry-instrumentation-starlette==0.21b0
opentelemetry-instrumentation-tornado==0.21b0
opentelemetry-instrumentation-wsgi==0.21b0
opentelemetry-instrumentation-aiohttp-client==0.22b0
opentelemetry-instrumentation-asgi==0.22b0
opentelemetry-instrumentation-asyncpg==0.22b0
opentelemetry-instrumentation-boto==0.22b0
opentelemetry-instrumentation-botocore==0.22b0
opentelemetry-instrumentation-celery==0.22b0
opentelemetry-instrumentation-dbapi==0.22b0
opentelemetry-instrumentation-django==0.22b0
opentelemetry-instrumentation-elasticsearch==0.22b0
opentelemetry-instrumentation-fastapi==0.22b0
opentelemetry-instrumentation-falcon==0.22b0
opentelemetry-instrumentation-flask==0.22b0
opentelemetry-instrumentation-grpc==0.22b0
opentelemetry-instrumentation-jinja2==0.22b0
opentelemetry-instrumentation-mysql==0.22b0
opentelemetry-instrumentation-psycopg2==0.22b0
opentelemetry-instrumentation-pymemcache==0.22b0
opentelemetry-instrumentation-pymongo==0.22b0
opentelemetry-instrumentation-pymysql==0.22b0
opentelemetry-instrumentation-pyramid==0.22b0
opentelemetry-instrumentation-redis==0.22b0
opentelemetry-instrumentation-requests==0.22b0
opentelemetry-instrumentation-sqlalchemy==0.22b0
opentelemetry-instrumentation-sqlite3==0.22b0
opentelemetry-instrumentation-starlette==0.22b0
opentelemetry-instrumentation-tornado==0.22b0
opentelemetry-instrumentation-wsgi==0.22b0
12 changes: 6 additions & 6 deletions python/src/otel/otel_sdk/requirements.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
opentelemetry-api==1.2.0
opentelemetry-sdk==1.2.0
opentelemetry-exporter-otlp==1.2.0
opentelemetry-distro==0.21b0
opentelemetry-instrumentation==0.21b0
opentelemetry-sdk-extension-aws==0.21b0
opentelemetry-api==1.3.0
opentelemetry-sdk==1.3.0
opentelemetry-exporter-otlp==1.3.0
opentelemetry-distro==0.22b0
opentelemetry-instrumentation==0.22b0
opentelemetry-sdk-extension-aws==0.22b0
10 changes: 5 additions & 5 deletions python/src/otel/tests/test-requirements.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
opentelemetry-api==1.2.0
opentelemetry-sdk==1.2.0
opentelemetry-distro==0.21b0
opentelemetry-instrumentation==0.21b0
opentelemetry-sdk-extension-aws==0.21b0
opentelemetry-api==1.3.0
opentelemetry-sdk==1.3.0
opentelemetry-distro==0.22b0
opentelemetry-instrumentation==0.22b0
opentelemetry-sdk-extension-aws==0.22b0
pytest