aws-lambda: handler never wrapped with directory index handler

[mooyoul]

What version of OpenTelemetry are you using?

• @opentelemetry/api - 1.0.3
• @opentelemetry/instrumentation - 0.25.0
• @opentelemetry/sdk-node - 0.25.0
• @opentelemetry/instrumentation-aws-lambda - 0.26.0

What version of Node are you using?

Lambda Node.js Runtime (Currently Node.js v14.17.4)

What did you do?

Use directory index as handler location

What did you expect to see?

Handler should be instrumented

What did you see instead?

Handler never instrumented

Additional context

Current aws-lambda instrumentation assumes that given handler is file.
For example, If api.handler is given, instrumentation expects given handler is located at /var/task/api.js.

This breaks instrumentation working with directory index. If actual api module is located at /var/task/api/index.js, Hander never wrapped because it will not match to /var/task/api.js.

Possible workaround for this is use require.resolve to resolve module path, or create additional InstrumentationNodeModuleDefinition for directory index. I'm not sure which way is reasonable so please share your thoughts then i will make some changes and raise a PR.

[mooyoul]

Ping @willarmiros @NathanielRN

[willarmiros]

Can you point to where in the instrumentation code this resolution is occurring? I think require.resolve makes sense to me as long as it doesn't break existing behavior.

Also just want to confirm, this wasn't a regression correct? E.g. not a new behavior with instrumentation v0.26.0?

[mooyoul]

Hello @willarmiros, Thank you for quick response!

Can you point to where in the instrumentation code this resolution is occurring?

Here it is: https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-aws-lambda/src/instrumentation.ts#L99-L104

I think require.resolve makes sense to me as long as it doesn't break existing behavior.

Agree. Module resolution is quite complex (See this pseudo code: https://nodejs.org/api/modules.html#modules_all_together)

One caveats with this is that require.resolve will throw an MODULE_NOT_FOUND Error if there are no matching module. I'm not sure throwing an Error in instrumentation init method is safe or not.

Also just want to confirm, this wasn't a regression correct? E.g. not a new behavior with instrumentation v0.26.0?

As far i know, that business logic was changed about 6 month ago. I don't think that bug is introduced from v0.26.0.

[mooyoul]

I would like to share results:

Passing "resolved file path" as name of InstrumentationNodeModuleDefinition fixes this issue. also modifying name of InstrumentationNodeModuleFile is required. because if handler is directory index, actual module name will be index.

but there's another issue: open-telemetry/opentelemetry-js#2450 is not available in current stable release. @dyladan Is there any opportunity to release patch release of @open-telemetry/instrumentation? It seems that open-telemetry/opentelemetry-js#2505 does not include experimental package.

[dyladan]

Experimental will be released immediately after stable. The reason is that there may be changes in the stable packages which need to be incorporated in the experimental packages.

[dyladan]

open-telemetry/opentelemetry-js#2512

[github-actions]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[dyladan]

@mooyoul there has been a release. Is this issue now fixed?

[mooyoul]

@dyladan Unfortunately, No. This bug still exists in latest version of aws-lambda instrumentation. but I have a fix and will propose a PR :)

[github-actions]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[github-actions]

This issue was closed because it has been stale for 14 days with no activity.

[dyladan]

@mooyoul were you ever able to open that PR?

[github-actions]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[github-actions]

This issue was closed because it has been stale for 14 days with no activity.