feat(cli): Add a CLI to build deep-links to Central ELK #36

akash1810 · 2023-12-12T20:08:11Z

What does this change?

Adds a (Deno) CLI tool to build a deep-link to Central ELK, and open it in user's browser.

How to test

Checkout the branch
From the cli directory, run deno task demo to view the logs for Riff-Raff PROD
OR from the cli directory, run deno run main.ts --help and follow the instructions
To run the binary, from the cli directory, run deno task compile then run ./dist/devx-logs --help and follow the instructions

Notes

We don't yet have a defined vulnerability scanning story for the Deno ecosystem, so I've deliberately opted to use the standard library, and no third-party dependencies (cc @guardian/devx-security).

Adds a (Deno) CLI tool to build a deep-link to Central ELK, and open it in user's browser.

See guardian/devx-logs#36

Requires guardian/homebrew-devtools#96.

.github/workflows/release-cli.yml

akash1810 · 2023-12-13T11:12:36Z

cli/deno.json

@@ -0,0 +1,6 @@
+{
+  "tasks": {
+    "compile": "deno compile --allow-run=open --target aarch64-apple-darwin --output dist/devx-logs index.ts",


Only compile for M series macs, because as a department, that's what we use for local development.

The resulting binary is pretty big for what it does (>100MB). We could use bash instead of Deno, however we'd lose type-safety etc.

It's because the Deno runtime has to be shipped within the binary. Can't be helped unfortunately (:

If we're compiling to a specific arch anyway, is it worth using a compiled language like Rust or Go instead? Would likely cut down on the runtime size significantly.

Or compile to JS and run with the users existing node binaries?

If we're compiling to a specific arch anyway, is it worth using a compiled language like Rust or Go instead? Would likely cut down on the runtime size significantly.

Chose TS on the assumption that it's the most familiar across the department (other than possibly Scala). If we move to JS, we could probably distribute this via NPM, rather than homebrew?

WDYT?

I haven't worked with it much, but deno install could be a nice middle-ground, which wouldn't require much change to the code? https://docs.deno.com/runtime/manual/tools/script_installer

@bryophyta my understanding is that install would create a script that invokes the CLI using deno. That means that people would have to have deno installed in order to run the CLI, since the runtime is not included.
I don't think we should require people to install additional software when running CLIs (which is why I'm personally averse to CLIs that need, for example, the JVM to run). I would rather have a 100MB standalone executable that's easy to distribute - I don't see it as different from a huge Electron app (which we've pretty much accepted as the norm).
As Ash says, there are languages there are better suited to writing CLIs, but Deno is a good compromise imho, since TypeScript is pretty much understood by everyone at the G.

We distribute our dev tooling via homebew. I'd be keen for this to also be distributed the same way, to reduce cognitive load. Homebrew formulae supports the idea of dependencies, so we could craft this such that the raw TS files form the Homebrew artifact, and we have a dependency on Deno. Theoretically this would provide the same benefit as distributing a binary - the reduction of "works on my machine" symptoms.

I'd suggest we ship as is, and revise our approach once we understand what to optimise for (disk usage, performance, maintainability, etc.). Our work laptops have quite a lot of storage (1TB), and I'd be surprised if ~100MB is noticeable.

WDYT?

cli/deno.lock

cli/index.ts

.github/workflows/release-cli.yml

cli/index.ts

See https://docs.deno.com/runtime/manual/references/contributing/style_guide#do-not-use-the-filename-indextsindexjs.

See https://docs.deno.com/runtime/manual/references/contributing/style_guide#each-module-should-come-with-a-test-module.

mchv · 2023-12-14T09:34:40Z

@akash1810 I think you may be missing changing SKIP_NODE to true in snyk.yml to have the client code scanned.

akash1810 · 2023-12-14T10:00:26Z

@akash1810 I think you may be missing changing SKIP_NODE to true in snyk.yml to have the client code scanned.

This CLI is written with Deno, rather than Node. AFAICT Deno isn't yet supported by Snyk.

This'll allow `main.ts` to be unit tested.

AshCorr · 2023-12-19T11:11:18Z

cli/transform.ts

+export function removeUndefined(
+  obj: Record<string, string | undefined>,
+): Record<string, string> {
+  return Object.entries(obj).filter(([, value]) => !!value).reduce(


I think this would stop you for filtering for any values that are 0?

!!0 == false !!1 == true

You're correct, however removeUndefined takes a string, and !!"0" is true... because JavaScript...

I've added a unit test for this now too - 824038d.

See guardian/devx-logs#36

akash1810 force-pushed the aa/cli branch 5 times, most recently from 7742d51 to 6f5f177 Compare December 12, 2023 23:36

akash1810 added 2 commits December 13, 2023 08:15

feat(cli): Add a CLI to build deep-links to Central ELK

8398944

Adds a (Deno) CLI tool to build a deep-link to Central ELK, and open it in user's browser.

fix(cli): Compile for M1 Macs

54d8357

akash1810 force-pushed the aa/cli branch 7 times, most recently from 516affe to e87a6cb Compare December 13, 2023 08:40

akash1810 added a commit to guardian/homebrew-devtools that referenced this pull request Dec 13, 2023

feat: Add a new formula for DevX Logs

d3e4dff

See guardian/devx-logs#36

akash1810 mentioned this pull request Dec 13, 2023

feat: Add a new formula for DevX Logs guardian/homebrew-devtools#96

Merged

akash1810 force-pushed the aa/cli branch 2 times, most recently from de07cae to 2fbfc97 Compare December 13, 2023 09:06

cd(cli): Release the CLI upon new tag

d4e9805

akash1810 force-pushed the aa/cli branch from 2fbfc97 to d4e9805 Compare December 13, 2023 09:08

akash1810 added a commit to guardian/homebrew-devtools that referenced this pull request Dec 13, 2023

feat: Add a new formula for DevX Logs

5858f63

See guardian/devx-logs#36

docs(cli): Add installation instructions

a60705e

Requires guardian/homebrew-devtools#96.

akash1810 commented Dec 13, 2023

View reviewed changes

.github/workflows/release-cli.yml Show resolved Hide resolved

akash1810 marked this pull request as ready for review December 13, 2023 11:01

akash1810 requested a review from a team as a code owner December 13, 2023 11:01

akash1810 commented Dec 13, 2023

View reviewed changes

.github/workflows/release-cli.yml Show resolved Hide resolved

akash1810 commented Dec 13, 2023

View reviewed changes

cli/index.ts Outdated Show resolved Hide resolved

akash1810 commented Dec 13, 2023

View reviewed changes

cli/index.ts Outdated Show resolved Hide resolved

marsavar reviewed Dec 13, 2023

View reviewed changes

cli/index.ts Outdated Show resolved Hide resolved

akash1810 force-pushed the aa/cli branch 3 times, most recently from 4193945 to ef05394 Compare December 13, 2023 18:31

akash1810 added 5 commits December 14, 2023 07:50

test(cli): Unit test the core logic

9131b16

refactor(cli): Remove dependency https://deno.land/x/[email protected]

1ee363e

refactor(cli): Simplify URL building by wrapping every part

56a84ba

style(cli): Follow Deno style for filenames

5cf85ac

See https://docs.deno.com/runtime/manual/references/contributing/style_guide#do-not-use-the-filename-indextsindexjs.

style(cli): Follow Deno style for test filenames

30b2567

See https://docs.deno.com/runtime/manual/references/contributing/style_guide#each-module-should-come-with-a-test-module.

akash1810 force-pushed the aa/cli branch from 6f44bc4 to 30b2567 Compare December 14, 2023 07:51

fix(cli): Run main function only when running file directly

87f929b

This'll allow `main.ts` to be unit tested.

akash1810 enabled auto-merge December 15, 2023 10:18

AshCorr reviewed Dec 19, 2023

View reviewed changes

akash1810 disabled auto-merge December 19, 2023 11:13

AshCorr approved these changes Dec 19, 2023

View reviewed changes

test: Unit test removeUndefined with "0" input

824038d

akash1810 merged commit 8a3cc09 into main Dec 19, 2023
3 checks passed

akash1810 deleted the aa/cli branch December 19, 2023 11:25

akash1810 added a commit to guardian/homebrew-devtools that referenced this pull request Dec 19, 2023

feat: Add a new formula for DevX Logs

37cd1cc

See guardian/devx-logs#36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cli): Add a CLI to build deep-links to Central ELK #36

feat(cli): Add a CLI to build deep-links to Central ELK #36

akash1810 commented Dec 12, 2023 •

edited

Loading

akash1810 Dec 13, 2023

akash1810 Dec 13, 2023 •

edited

Loading

marsavar Dec 13, 2023

AshCorr Dec 13, 2023 •

edited

Loading

akash1810 Dec 13, 2023 •

edited

Loading

bryophyta Dec 13, 2023

marsavar Dec 13, 2023 •

edited

Loading

akash1810 Dec 14, 2023

mchv commented Dec 14, 2023

akash1810 commented Dec 14, 2023 •

edited

Loading

AshCorr Dec 19, 2023

akash1810 Dec 19, 2023 •

edited

Loading

feat(cli): Add a CLI to build deep-links to Central ELK #36

feat(cli): Add a CLI to build deep-links to Central ELK #36

Conversation

akash1810 commented Dec 12, 2023 • edited Loading

What does this change?

How to test

Notes

akash1810 Dec 13, 2023

Choose a reason for hiding this comment

akash1810 Dec 13, 2023 • edited Loading

Choose a reason for hiding this comment

marsavar Dec 13, 2023

Choose a reason for hiding this comment

AshCorr Dec 13, 2023 • edited Loading

Choose a reason for hiding this comment

akash1810 Dec 13, 2023 • edited Loading

Choose a reason for hiding this comment

bryophyta Dec 13, 2023

Choose a reason for hiding this comment

marsavar Dec 13, 2023 • edited Loading

Choose a reason for hiding this comment

akash1810 Dec 14, 2023

Choose a reason for hiding this comment

mchv commented Dec 14, 2023

akash1810 commented Dec 14, 2023 • edited Loading

AshCorr Dec 19, 2023

Choose a reason for hiding this comment

akash1810 Dec 19, 2023 • edited Loading

Choose a reason for hiding this comment

akash1810 commented Dec 12, 2023 •

edited

Loading

akash1810 Dec 13, 2023 •

edited

Loading

AshCorr Dec 13, 2023 •

edited

Loading

akash1810 Dec 13, 2023 •

edited

Loading

marsavar Dec 13, 2023 •

edited

Loading

akash1810 commented Dec 14, 2023 •

edited

Loading

akash1810 Dec 19, 2023 •

edited

Loading