Skip to content

Update cli packages where necssary #293

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

Closed
simitt opened this issue Aug 26, 2022 · 7 comments · Fixed by #301
Closed

Update cli packages where necssary #293

simitt opened this issue Aug 26, 2022 · 7 comments · Fixed by #301
Labels
aws-λ-extension AWS Lambda Extension
Milestone

Comments

@simitt
Copy link
Contributor

simitt commented Aug 26, 2022

see https://github.com/elastic/apm-aws-lambda/security/dependabot

@github-actions github-actions bot added the aws-λ-extension AWS Lambda Extension label Aug 26, 2022
@simitt simitt added this to the 8.5 milestone Aug 26, 2022
@simitt simitt changed the title Update cli packages to satisfy dependabot Update cli packages where necssary Aug 31, 2022
@axw
Copy link
Member

axw commented Sep 8, 2022

I started looking at this, and I'm wondering if we should either trim back or remove the CLI altogether. The CLI can do the following things:

  • build & publish the layer
  • update a Lambda function with a layer by ARN
  • update environment variables for a function
  • install a Lambda function
  • profile a Lambda function

Long term I think we'll be better served by separating these. I would also prefer to not have multiple ways of doing the same thing, as it makes maintenance more difficult.

We are already working on adding Terraform for declaratively creating the extension layer, and installing a function with a layer (#296).

I think profiling/benchmarking is something we should do regularly, so that's something we should also aim to automate. This should be running close to Lambda function to minimise network overhead (and cost!), so ideally within the same AWS zone. We might also want to send load from multiple clients.

On the whole, I'm leaning towards removing the CLI altogether. @simitt @kruskall WDYT?

@simitt
Copy link
Contributor Author

simitt commented Sep 8, 2022

I don't really use the CLI when testing anything, but I am not aware of general adoption rate. @akhileshpok do you have any insights on whether customers are using it?
In general I agree with rather having targeted tooling where needed. The CLI is marked as experimental. If there are no signs of (heavy) CLI usage, I'd be fine with removing it alltogether.

@axw
Copy link
Member

axw commented Sep 9, 2022

I would also point out that our docs do not mention the CLI, and we provide instructions for using well known tools (AWS CLI, SAM, Serverless, Terraform) for using the layers: https://www.elastic.co/guide/en/apm/agent/nodejs/current/lambda.html#_step_3_configure_apm_on_aws_lambda.

@akhileshpok
Copy link

@axw - Is the purpose of this issue to remove ability to install or configure AWS Lambda Extn. via CLI, as described here; https://www.elastic.co/guide/en/apm/agent/java/current/aws-lambda.html? I can check whether removing this functionality will cause any issues with a couple of existing customers and revert back to you, as I don't have a particular opinion either ways.

@kruskall
Copy link
Member

kruskall commented Sep 9, 2022

I started looking at this, and I'm wondering if we should either trim back or remove the CLI altogether. The CLI can do the following things:

* build & publish the layer

* update a Lambda function with a layer by ARN

* update environment variables for a function

* install a Lambda function

* profile a Lambda function

Long term I think we'll be better served by separating these. I would also prefer to not have multiple ways of doing the same thing, as it makes maintenance more difficult.

We are already working on adding Terraform for declaratively creating the extension layer, and installing a function with a layer (#296).

I think profiling/benchmarking is something we should do regularly, so that's something we should also aim to automate. This should be running close to Lambda function to minimise network overhead (and cost!), so ideally within the same AWS zone. We might also want to send load from multiple clients.

On the whole, I'm leaning towards removing the CLI altogether. @simitt @kruskall WDYT?

+1 on removing the CLI. I don't think we should mix languages for testing/profiling as it adds more requirements and create friction for developers.

@astorm
Copy link
Contributor

astorm commented Sep 9, 2022

Just for historical context -- I presume you'all are talking about the Node.js CLI application located here: https://github.com/elastic/apm-aws-lambda/tree/main/cli and not using the aws cli command for installation that @akhileshpok mentioned above.

This was a tool that developed out of necessity during the rapid prototying/initial-development of the Lambda extension and product. While there was some initial talk of developing these capabilities further for customers (specifically an installer) its primary users were other elasticians who needed to do things like build & publish the layer, update a Lambda function with a layer by ARN, update environment variables for a function, install a Lambda function, or profile a Lambda function during their day to day work on the Lambda extension or agent related tasks. Node.js was chosen over some other tech because the person writing the scripts was a Node.js developer and was a little burned out on Makefiles and bash scripts ;) (they were also me).

While the tool was shared informally with some early alpha-ish users as a quick way to build or install the layer, its functionality has long been supplanted by the builds that happen with each release (GitHub release, the AWS layer, the aforementioned docs).

At the risk of stepping on @akhileshpok's toes and putting my unofficial Lambda TPM hat back on I'd say you'all are safe removing this from the repository. If there are customers who are using it for something it will still be available via the older release tags (removing it won't significantly disrupt folks) and I'd wager if they were intrepid enough to find it hanging out in the /cli folder they'll be intrepid enough to open issues if they find it's gone. At that point you'all can assess their needs and whether it's something to bring back or something to solve in a different way.

@axw
Copy link
Member

axw commented Sep 12, 2022

@astorm thanks for weighing in, and for providing the historical context!

Is the purpose of this issue to remove ability to install or configure AWS Lambda Extn. via CLI, as described here; https://www.elastic.co/guide/en/apm/agent/java/current/aws-lambda.html? I can check whether removing this functionality will cause any issues with a couple of existing customers and revert back to you, as I don't have a particular opinion either ways.

@akhileshpok no, I'm not proposing to remove anything that's currently documented. The CLI that's documented there is the AWS CLI -- this is provided by AWS, and is often used by folks who develop on AWS.

What I'm referring to is this custom CLI: https://github.com/elastic/apm-aws-lambda/tree/main/cli. Given Alan's response above, I'm confident that we can remove it without any issues. If anyone external to Elastic is relying on it, then we can either reinstate it, or work with those users to find an alternative.

@axw axw closed this as completed in #301 Sep 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws-λ-extension AWS Lambda Extension
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants