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

[ws-manager] add metrics to track initialize and finalize of workspaces #9355

Merged
merged 1 commit into from
Apr 18, 2022

Conversation

sagor999
Copy link
Contributor

@sagor999 sagor999 commented Apr 15, 2022

Description

Add metrics to track initialize (time it takes to download and untar backup for example or to load prebuild) and finalize (time it takes to create and upload backup of workspace).
This metrics will be used later on to compare them to PVC implementation.

Related Issue(s)

Fixes #9353

How to test

Spin up new workspace preview environment using installer from this PR
Create new workspace, create large file in workspace and stop workspace.
Then open that workspace again.
Port forward metrics port of ws-manager and look for new metrics.
Times reported will match the time it took to finalize or initialize the workspace.

Release Notes

[ws-manager] add metrics to track initialize and finalize of workspaces

Documentation

@sagor999 sagor999 marked this pull request as ready for review April 15, 2022 19:32
@sagor999 sagor999 requested a review from a team April 15, 2022 19:32
@github-actions github-actions bot added the team: workspace Issue belongs to the Workspace team label Apr 15, 2022
@@ -916,6 +922,7 @@ func (m *Monitor) finalizeWorkspaceContent(ctx context.Context, wso *workspaceOb
backupError error
gitStatus *csapi.GitStatus
)
t := time.Now()
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better that calculate the start time at finalizeWorkspaceContent and have a defer function to calculate the end time? For example:

func (m *Monitor) finalizeWorkspaceContent(ctx context.Context, wso *workspaceObjects) {
    t := time.Now()
    ...
    tpe, err := wso.WorkspaceType()
    if err != nil {
        tracing.LogError(span, err)
        log.WithError(err).Warn("cannot determine workspace type - assuming this is a regular")
        tpe = api.WorkspaceType_REGULAR
    }
    defer func() {
        wsType := api.WorkspaceType_name[int32(tpe)]
	hist, errHist := m.manager.metrics.finalizeTimeHistVec.GetMetricWithLabelValues(wsType)
	if errHist != nil {
		log.WithError(errHist).WithField("type", wsType).Warn("cannot get finalize time histogram metric")
	}
	hist.Observe(time.Since(t).Seconds())
    }()
    ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I thought of that, but that would catch incorrect metrics in case of this return statement:

if !didSometing {
// someone else is managing finalization process ... we don't have to bother
return
}

I only want to catch timing for workspaces where we actually did some work.

@roboquat roboquat merged commit f9c93c4 into main Apr 18, 2022
@roboquat roboquat deleted the pavel/9353 branch April 18, 2022 23:15
@roboquat roboquat added deployed: workspace Workspace team change is running in production deployed Change is completely running in production labels Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: workspace Workspace team change is running in production deployed Change is completely running in production release-note size/M team: workspace Issue belongs to the Workspace team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

observability: add metrics so we can measure frequency and duration of tar backup and restore now (with tar).
4 participants