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

feat(lambda): Connect CW Live Tail to Lambda functions #6423

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

valerena
Copy link
Contributor

Allow to "Tail Logs" from Lambda functions, which will get the configured Log Group for a particular function and start a Live Tail session in that Log Group.

Some code was removed, because with the recently updated version of the SDK v2, we can take the configured log group directly from the function's configuration and we don't need that extra call using SDK V3 that was being made inside deployedNode.ts

Problem

To start a CloudWatch Live Tail session, a user needs to know their Log Group name and find it in the AWS explorer. For Lambda users, that might not be easy to find or well known all the time

Solution

We help the Live Tail experience for Lambda customers by allowing them to start a Live Tail session directly from their function in the AWS explorer and in the Application Builder section for deployed functions, without having to search for their Log Group explicitly.


  • Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
  • Your code changes must meet the guidelines in CONTRIBUTING.md.
  • License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

Allow to "Tail Log Group" from Lambda functions, which will get the
configured Log Group for a particular function and start a Live Tail
session in that Log Group.

Because of the recently updated version of the SDK v2, we can take
the configured log group directly from the function's configuration
and we don't need an extra call using SDK V3 that was being made in
deployedNode.ts
@valerena valerena requested a review from a team as a code owner January 23, 2025 23:01
@@ -91,6 +91,7 @@
"AWS.command.appBuilder.deploy": "Deploy SAM Application",
"AWS.command.appBuilder.build": "Build SAM Template",
"AWS.command.appBuilder.searchLogs": "Search Logs",
"AWS.command.appBuilder.tailLogs": "Tail Logs",
Copy link
Contributor

@justinmk3 justinmk3 Jan 23, 2025

Choose a reason for hiding this comment

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

We should probably use the same wording (and actually the same AWS.foo.bar id, instead of multiple) in all places:

"AWS.command.cloudWatchLogs.searchLogGroup": "Search Log Group",
"AWS.command.cloudWatchLogs.tailLogGroup": "Tail Log Group",

However, not a blocker for this PR. Needs a decision about which wording to use in both places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The difference here is that the one you linked shows "when you have a log group selected" (therefore the wording is "tail log group" as in "tail this log group"), while this new one is when you have a function selected. Maybe this could be "Tail function logs" instead of just "Tail logs". The inspiration was the other existing string right above "Search Logs" (with its corresponding "Search Log Group"), which is a similar behavior, but that starts a search instead of using live tail.

Let me know if this difference makes sense or if we should still try to unify the wording.

Copy link
Contributor

Choose a reason for hiding this comment

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

The inspiration was the other existing string right above "Search Logs"

Yup I noticed that too, and I like the brevity of "Logs" instead of "Log Group". For now let's leave it as you have in this PR.

throw ToolkitError.chain(
err,
`Unable to fetch logs. Log group for function '${functionConfiguration!.FunctionName}' does not exist. ` +
'Invoking your function at least once will create the log group.'
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice hint! Very helpful to surface hints like this.

@justinmk3
Copy link
Contributor

Some code was removed, because with the recently updated version of the SDK v2, we can take the configured log group directly from the function's configuration and we don't need that extra call using SDK V3 that was being made inside deployedNode.ts

Thanks for noticing that, eliminating code is very helpful.

Co-authored-by: Justin M. Keyes <jmkeyes@amazon.com>
} catch (err) {
if (isError(err as Error, 'ResourceNotFoundException', "LogGroup doesn't exist.")) {
// If we caught this error, then we know `functionConfiguration` actually has a value
throw ToolkitError.chain(
Copy link
Member

Choose a reason for hiding this comment

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

will we get this issue when we deploy the function for the first time, and hit taillog before invoking it?

Copy link
Member

Choose a reason for hiding this comment

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

For I feel this should be a valid use case

const logGroupPrefix = '/aws/lambda/'
return configuration.logGroupName || logGroupPrefix + configuration.FunctionName
return configuration.LoggingConfig?.LogGroup || logGroupPrefix + configuration.FunctionName
Copy link
Member

Choose a reason for hiding this comment

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

is this because SDK update?

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.

3 participants