feat: add deploy insights

[jLopezbarb]

Proposed changes

DEV-201

This PR adds a k8s event whenever a build is triggered using a manifest v2 either if it's called from an okteto build command or a okteto deploy command.

The event follows the following schema:

{
	"$schema": "http://json-schema.org/draft-07/schema#",
	"type": "object",
	"properties": {
	  "devenv_name": {
		"type": "string",
		"description": "Name of the development environment"
	  },
	  "repository": {
		"type": "string",
		"format": "uri",
		"description": "URL of the code repository"
	  },
	  "namespace": {
		"type": "string",
		"description": "Namespace where the deployment will be created"
	  },
	  "phases": {
		"type": "array",
		"description": "An array of the phases to be executed",
		"items": {
		  "type": "object",
		  "properties": {
			"name": {
			  "type": "string",
			  "description": "Name of the phase or the index of the command in the array"
			},
			"duration": {
			  "type": "integer",
			  "description": "Execution time of the command in seconds"
			}
		  },
		  "required": ["name", "idx", "duration"]
		}
	  },
	  "success": {
		"type": "boolean",
		"description": "Whether the overall process was successful"
	  },
	  "schema_version": {
		"type": "string",
		"description": "Version of the schema"
	  
	  }
	},
  }

How to validate

1. Run okteto deploy (--remote) (--wait) on okteto/movies
2. Run kubectl get events.events.k8s.io -w
3. Check that there is a event created after the deploy with all the times

CLI Quality Reminders 🔧

For both authors and reviewers:

• Scrutinize for potential regressions
• Ensure key automated tests are in place
• Build the CLI and test using the validation steps
• Assess Developer Experience impact (log messages, performances, etc)
• If too broad, consider breaking into smaller PRs
• Adhere to our code style and code review guidelines

[derek]

Thank you for your contribution. unfortunately, one or more of your commits are missing the required "Signed-off-by:" statement. Signing off is part of the Developer Certificate of Origin (DCO) which is used by this project.

Read the DCO and project contributing guide carefully, and amend your commits using the git CLI. Note that this does not require any cryptography, keys or special steps to be taken.

💡 Shall we fix this?

This will only take a few moments.

First, clone your fork and checkout this branch using the git CLI.

Next, set up your real name and email address:

git config --global user.name "Your Full Name"
git config --global user.email "you@domain.com"

Finally, run one of these commands to add the "Signed-off-by" line to your commits.

If you only have one commit so far then run: git commit --amend --signoff and then git push --force.
If you have multiple commits, watch this video.

Check that the message has been added properly by running "git log".

[codecov]

Codecov Report

Merging #4206 (46143ee) into master (4aeee10) will increase coverage by 0.07%.
Report is 2 commits behind head on master.
The diff coverage is 61.53%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4206      +/-   ##
==========================================
+ Coverage   45.62%   45.69%   +0.07%     
==========================================
  Files         302      303       +1     
  Lines       27440    27535      +95     
==========================================
+ Hits        12520    12583      +63     
- Misses      13856    13883      +27     
- Partials     1064     1069       +5     

[ifbyol]

Would it make sense to encode the data to base64 as we do with other data like variables?

[jLopezbarb]

I don't think it's necessary given that this data is not going to grow too much in the future and from the k8s spec it should be human readable.

[ifbyol]

It is not about the length, it is about the format and possible issues with chars or similar. As we are controlling the format of that, there might not be issues, but just in case. Anyways, this is completely a nit, you can ignore it

[jLopezbarb]

Right now it's a json that we generate. We'll have to rethink it if we add the commands names but right now we are the ones that set all the naming in the json generated

[ifbyol]

For the future, out of scope of this PR. If we have 2 different implementations to satisfy an interface and some of the functions have duplicated the logic, it is error prone. We should extract those implementations to a common piece and the implementations should use that common piece through struct embedding or dependency

[ifbyol]

@jLopezbarb I think you need to merge master. There are changes not sure they should appear (apart from the conflicts)

[ifbyol]

Was all the namespace calculation done before? I mean, do we consider here if the namespace is specified as command flag?

[jLopezbarb]

Yes, at the beginning of the command the okteto context set the namespace in the global variable of okteto contextStore which is the one we are using. When we read the manifest we set it on the options.Manifest and use it for the rest of the command.

Why we can't use okteto.GetContext().Namespace?

If the namespace is specified in the manifest we are not setting the global variable, that's why we need this code

[ifbyol]

Backend translations of the configmap doesn't know anything about phase field and the Data field is recreated each time. Would there be any scenario in which we might lose the information in the configmap because of that? have we thought about that?

[jLopezbarb]

Given that right now we are only adding the phases field in order to track events and they are calculated and created in K8s in the CLI execution, there is no scenario where we might lose the information.

If we wanted to store the information to show it on the UI, there is one scenario that we have to take a look (deploying with a previous CLI version)

[jLopezbarb]

Given that right now we are only adding the phases field in order to track events and they are calculated and created in K8s in the CLI execution, there is no scenario where we might lose the information.

If we wanted to store the information to show it on the UI, there is one scenario that we have to take a look (deploying with a previous CLI version)

[ifbyol]

Would there be an scenario in which a deploy within another deploy would use the same configmap, overriding the phase field for the main one?

[jLopezbarb]

Do you mean if they have the same name? It'll create two events with the same information besides the duration. What it will happen is:

1. Main deploy starts the timer:
2. The deploy from inside starts the timer
3. The deploy from inside updates the configmap
4. The deploy from inside sends the event with its duration
5. The main deploy updates the configmap
6. The main deploy sends the event with its duration

I think it's the way it must behave but open to discuss it

[ifbyol]

In the moment in which the main deploy add a phase before the "deploy inside" adds its own, the inside one would overwrite the phase field of the main one. But not sure if the scenario is possible or not

[ifbyol]

Anyways, this would be a corner case and not sure if it is even possible

[ifbyol]

I don't think we should change this function or at least we should change the name of the function. We are assuming that in all the places where we want the string representation of a repository URL we want to use https protocol. In some cases we might just want the URL as specified by the user.

Among other things, this is also being used by the function GetAnonymizedRepo, that is being used in different places. Have we double checked that in all the places where that is being used doesn't matter to change the URL scheme?

[jLopezbarb]

I have double checked. We use GetAnonymizedRepo in:

• build command for sending analytics, so with this change we'll be sending always the same scheme for the repo and have better metrics
• pipeline command in order to show a list of the repositories, which I think we should be showing all with the same schema also.
• In our info/debug logs to hide user information
• The new insights package

The only one that I have doubts is with the logs. It's only used in the inferer, but given that the comparison between two repositories URL takes that into account I think we are good to go