aws-samples / amazon-sagemaker-notebook-instance-lifecycle-config-samples

A collection of sample scripts to customize Amazon SageMaker Notebook Instances using Lifecycle Configurations
MIT No Attribution
422 stars 250 forks source link

Append metrics configurations to existing logging configurations #87

Closed apogupta2018 closed 2 years ago

apogupta2018 commented 2 years ago

Issue #, if available:

Description of changes: This change is to fix issue where customers are not seeing logs after running this LCC. Previously with AL1, there were two cloudwatch instances running where the old process had old logging configs and was publishing logs, while the newer process had the metrics configs and was publishing metrics. For AL2 instead of starting a second process, it is actually restarting the old process, which results in that new process only having metrics configs and not having any logging configs. As a result no logs are emitted. This change will append the new configs so the process can publish both the logs and the metrics.

Testing Done

# Provide your commands here
/you/commands/here

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

tdmalone commented 1 year ago

I think this PR introduces a regression - it removed the -s which starts the CloudWatch agent, so if both restart and systemctl commands fail (which they are on the Notebook Instance I'm running them on at the moment), the CloudWatch agent never starts (yet the lifecycle config still succeeds, due to the || true).

As a result, I'm getting no metrics in CloudWatch - until I put the -s back in (this is on notebook-al2-v1).

tayodok commented 1 year ago

Noted. I'll take a look and investigate