Skip to content
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

AWS lambda - configurable flush timeout #1960

Merged
merged 2 commits into from Jan 11, 2021
Merged

AWS lambda - configurable flush timeout #1960

merged 2 commits into from Jan 11, 2021

Conversation

ghost
Copy link

@ghost ghost commented Dec 23, 2020

Closes #1959

  • handler's flush timeout is configurable now
  • wrapper uses env prop to set the timeout (= 1 sec default)

@ghost
Copy link
Author

ghost commented Dec 23, 2020

@anuraaga please have a look

Copy link
Contributor

@anuraaga anuraaga left a comment

Choose a reason for hiding this comment

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

Thanks!

@ghost
Copy link
Author

ghost commented Dec 28, 2020

@anuraaga @trask @tylerbenson please merge as soon as convenient

@anuraaga
Copy link
Contributor

anuraaga commented Jan 5, 2021

@trask Can you doublecheck this too? Thanks!

@@ -6,6 +6,8 @@ This package contains libraries to help instrument AWS lambda functions in your
To use the instrumentation, configure `OTEL_LAMBDA_HANDLER` env property to your lambda handler method in following format `package.ClassName::methodName`
and use one of wrappers as your lambda `Handler`.

In order to configure a span flush timeout (default is set to 1 second), please configure `OTEL_LAMBDA_FLUSH_TIMEOUT` env property. The value is in seconds.
Copy link
Member

Choose a reason for hiding this comment

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

probably should follow standard config property naming? see #1414

Copy link
Author

Choose a reason for hiding this comment

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

That's a very good idea, I'm all in favour of standardisation. This is of course a bit different than the "regular" usage of the config properties as here we're not configuring an instrumentation but a runtime wrapper (only via env property) that uses the aws lambda instrumentation

If such usage fits into the model that you've proposed (for me it sounds OK), I believe that the updated property name should be: OTEL_INSTRUMENTATION_AWS-LAMBDA_FLUSH_TIMEOUT There is also another config property, defining the handler, which would then become: OTEL_INSTRUMENTATION_AWS-LAMBDA_HANDLER.

@trask please have a look ^^

Copy link
Member

Choose a reason for hiding this comment

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

that sounds good, with underscore instead of dash, e.g.

OTEL_INSTRUMENTATION_AWS_LAMBDA_FLUSH_TIMEOUT
and
OTEL_INSTRUMENTATION_AWS_LAMBDA_HANDLER

Copy link
Author

Choose a reason for hiding this comment

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

Ah yes, kebab-case is for dot-property names only. Silly me :) Updated PR, please have a look.

@trask trask merged commit 179b225 into open-telemetry:master Jan 11, 2021
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.

AWS lamba tracing handler - configure flush timeout
4 participants