feat(resource-detector-aws): allow synchronous lambda detection #1274
Conversation
|
|
| @@ -18,20 +18,20 @@ import { | |||
| Detector, | |||
| Resource, | |||
| ResourceDetectionConfig, | |||
| } from '@opentelemetry/resources'; | |||
| } from "@opentelemetry/resources"; | |||
There was a problem hiding this comment.
seems you should tell your editor to leave ' untouched :o)
There was a problem hiding this comment.
Seems to me it should be set at the repo level :D. But fair, will do
blumamir
changed the title
There was a problem hiding this comment.
Thank you for the PR and sorry for the late review.
Would this PR solve the issue once merged?
Adding a detectSync function will allow the lambda detector to be used in this case, but people that need to use other detectors in a lambda will still run into the same issue. I wonder if adding a new function to each non-async detector is the way to go here.
Once stable, this function will be part of the public API and we will not be able to change it, so I suggest we are careful with extending the detector API for specific packages
|
|
||
| /** | ||
| * The AwsLambdaDetector can be used to detect if a process is running in AWS Lambda | ||
| * and return a {@link Resource} populated with data about the environment. | ||
| * Returns an empty Resource if detection fails. | ||
| */ | ||
| export class AwsLambdaDetector implements Detector { | ||
| async detect(_config?: ResourceDetectionConfig): Promise<Resource> { | ||
| detectSync(_config?: ResourceDetectionConfig): Resource { |
There was a problem hiding this comment.
If we add this option for sync detection, we should probably also document it in the README so users are aware of it and how to leverage it.
|
I think open-telemetry/opentelemetry-js#3460 will resolve this usecase. If not, please feel free to reopen |
Which problem is this PR solving?
See #1273
When using asynchronous SDK init, the lambda might have executed before the SDK has terminated its init (as no top-level await is possible for any of the AWS node runtimes)
Our kubernetes services are usually long-lived, so if we miss a few traces at startup, it's not a problem. For lambdas it's a bit of a different story. I need to find a way to ensure that the init is synchronous.
As it turns out, the detector is actually synchronous, but just returns a Promise. If we can bypass that by exposing a synchronous method (and ditching the detectResources method at the same time at the application level), then everything becomes peachy :)
Short description of the changes
Added a synchronous method for lambda resource detection.
(PS: Sorry for the quote replacement, maybe we could add a .prettierrc somewhere)