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

chore: Add experimental test analytics [CFG-2157] #3857

Merged
merged 2 commits into from
Sep 28, 2022

Conversation

ofekatr
Copy link
Contributor

@ofekatr ofekatr commented Sep 22, 2022

What does this PR do?

  • Computes analytics for the experimental test flow.
  • Prints the analytics to the debug logs.
  • DOES NOT send the analytics.

Where should the reviewer start?

src/lib/iac/test/v2/analytics

How should this be manually tested?

  • Run snyk iac test --experimental -d
  • Ensure the printed analytics are the expected ones.

Any background context you want to provide?

  • The current implementation is discussed in this Notion page. The page also describes the analytics that should be computed by the experimental flow, and some ideas for more analytics information that could be useful in the future.

  • Because the analytics information that we are going to compute cannot be sent over the network yet, it will be sufficient to print them to a debug logger for the time being. The information we compute must not be shared with the analytics module in the CLI.

  • This feature must be implemented in snyk and not in snyk-iac-test, because when we will eventually send the analytics over the network, we must use the analytics package as implemented in the CLI.

IaC analytics example
{
  "iac-type": {
      "armconfig": {
          "count": 1,
          "resource-count": 3
      },
      "cloudformationconfig": {
          "count": 2,
          "resource-count": 13
      },
      "terraformconfig": {
          "count": 10,
          "resource-count": 26,
          "low": 27,
          "high": 22,
          "medium": 10
      },
      "k8sconfig": {
          "count": 4,
          "resource-count": 5
      }
  },
  "packageManager": [
      "armconfig",
      "cloudformationconfig",
      "terraformconfig",
      "k8sconfig"
  ],
  "iac-issues-count": 59,
  "iac-ignored-issues-count": 0,
  "iac-files-count": 17,
  "iac-resources-count": 47,
  "iac-test-binary-version": "0.31.3"
}

What are the relevant tickets?

@ofekatr ofekatr changed the title Chore/add experimental test analytics cfg 2157 chore: Add experimental test analytics [CFG-2157] Sep 22, 2022
@ofekatr ofekatr marked this pull request as ready for review September 22, 2022 12:19
@ofekatr ofekatr requested a review from a team as a code owner September 22, 2022 12:19
Copy link
Contributor

@ipapast ipapast left a comment

Choose a reason for hiding this comment

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

looks good on my local run and the numbers add up.
image

I assume that the iac-rules-bundle-version will be added later as per comment and we didn't want to default it to empty string, etc.

What about this comment/suggestion here? do we want to add the errors here too?

@ofekatr ofekatr force-pushed the chore/add-experimental-test-analytics-cfg-2157 branch 3 times, most recently from 216d19f to f1b5ff2 Compare September 28, 2022 08:29
@ofekatr
Copy link
Contributor Author

ofekatr commented Sep 28, 2022

What about this comment/suggestion here? do we want to add the errors here too?

@ipapast I've just added an iac-error-codes property to address it, I think it captures what the comment suggested, but please share your feedback too.

@ofekatr ofekatr force-pushed the chore/add-experimental-test-analytics-cfg-2157 branch from f1b5ff2 to 2e15e33 Compare September 28, 2022 08:32
@ofekatr ofekatr force-pushed the chore/add-experimental-test-analytics-cfg-2157 branch from 2e15e33 to e8794de Compare September 28, 2022 08:33
@ipapast
Copy link
Contributor

ipapast commented Sep 28, 2022

What about this comment/suggestion here? do we want to add the errors here too?

@ipapast I've just added an iac-error-codes property to address it, I think it captures what the comment suggested, but please share your feedback too.

I think it's very useful, let's keep it

@ofekatr ofekatr merged commit 2831b80 into master Sep 28, 2022
@ofekatr ofekatr deleted the chore/add-experimental-test-analytics-cfg-2157 branch September 28, 2022 13:11
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.

2 participants