chore: add the error string code to analytics [CC-786]

[teodora-sandu]

• Follows CONTRIBUTING rules

What does this PR do?

This PR defines error string codes for IaC errors and includes them in the analytics, to improve error readability.

Where should the reviewer start?

• src/cli/commands/test/iac-local-execution/error-utils.ts: new utility function for creating the error string code from the error type
• src/cli/commands/test/iac-local-execution/types.ts: comment letting people know if they add a new error, the name of the error will be used for error string codes
• src/cli/commands/test/index.ts and src/cli/index.ts: code for adding the new field to the analytics
• the rest is adding the strCode field to IaC custom errors using the utility function from src/cli/commands/test/iac-local-execution/error-utils.ts

How should this be manually tested?

1. Run npm run build
2. Run a failing scan: node ./dist/cli iac test test/fixtures/iac/kubernetes/pod-invalid.yaml --experimental -d
3. Notice the error-str-code being included under the metadata field.

Any background context you want to provide?

What are the relevant tickets?

• Jira ticket CC-786

Screenshots

[github-actions]

	Messages
📖	This PR will not trigger a new version. It doesn't include any commit message with feat or fix.

Generated by 🚫 dangerJS against 5dc0140

[ipapast]

Love it! Left a non-blocking comment for the constant case.

[jan-stehlik]

Hey @teodora-sandu , nice work 🎉 I have 2 smaller non-blocking comments below. I also have a bigger comment/question - I think we are missing tests for this entire implementation? what if someone horribly modifies the behaviour, would any automated test safeguard against that?

[jan-stehlik]

Not a huge fan of introducing 3rd party library into our already polluted repo. From what I understand we need a way to have value in format MISSING_REQUIRED_FIELDS_IN_KUBERNETES_YAML_ERROR. I think we have 2 decent options:

1. create enum with keys in upper case delimetered by underscore in types.ts denoted with capital letters as is convention for constants and keep the original enum IaCErrorCodes

export enum IAC_ERROR_CODES {
  MISSING_REQUIRED_FIELDS_IN_KUBERNETES_YAML_ERROR = 1031,
  // ...
}

2. create new enum just like above and delete original one. Make sure to replace all the occurences. This seems like a better option but is more laborious

The reason why i'm raising this is because dependencies in snyk cli got extremely out of hand (not to mention performance) and we got into habit to install library for any small job that can be easily written by us.

This is, however, not a blocking comment, just imo opportunity to improve a solution

[teodora-sandu]

The reasoning behind using this getErrorStringCode function is that we don't want to duplicate the work to add a new error (i.e. have to add an error code to IaCErrorCodes and a new error string code to some other new enum). And we don't want to remove the IaCErrorCodes enum because we want to have both error codes and readable error string codes. This function allows us to create new errors much more easily and not have to think about what error string code to give it, typos, etc.

The added package is pretty small https://bundlephobia.com/result?p=lodash.uppercase@4.3.0 in my opinion, as it only exposes the lodash _.upperCase method. So while we could have done this without this new package, I would say the benefits outweigh the added bundle size. But let me know if you disagree

[teodora-sandu]

I've re-implemented this function using built-in JavaScript methods, so we're not importing lodash.uppercase anymore 😃

[jan-stehlik]

nitpick: I wonder if we can standardise the way to submit analytics keys. We use camelCase in some places, snake_case in others and lowercase-delimitered-with-dash here. I would prefer if we used standardised approach everywhere (ideally camelCase or snake_case rather than inventing new case). What do you think?

Suggested change

	analytics.add('error-code', error.code);
	analytics.add('error-str-code', error.strCode);
	analytics.add('errorCode', error.code);
	analytics.add('errorStringCode', error.strCode);

[teodora-sandu]

I think this would require cooperation from multiple teams, as these fields might be used either by their Grafana dashboards or other dashboards/analytics to capture trends. I agree that it'd be better if we standardised this, but probably needs a wider discussion and is not a change that belongs to this PR

[teodora-sandu]

Thank you for your comprehensive, in-depth review @jan-stehlik! I've added two unit tests for the new getErrorStringCode function.

[jan-stehlik]

LGTM, nice work @teodora-sandu 💯

[jan-stehlik]

😍