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

[server] Reliably set log the value for volumeSnapshotId #12056

Merged
merged 2 commits into from
Aug 12, 2022

Conversation

kylos101
Copy link
Contributor

@kylos101 kylos101 commented Aug 11, 2022

Description

Set the value for volumeSnapshotId one time.

Background: we're trying to log/capture the above field. Why? The problem is, sometimes a feature (PVC) is enabled for workspace start in production, and we do not know why. We added logging to learn more, but, it is missing this field volSnapshotId: volumeSnapshotId,.

Related Issue(s)

Related to #11823

How to test

Start a workspace

Release Notes

NONE

Documentation

Werft options:

  • /werft with-preview

@kylos101
Copy link
Contributor Author

kylos101 commented Aug 11, 2022

/werft run

👍 started the job as gitpod-build-kylos101-fix-pvc-logging.1
(with .werft/ from main)

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-kylos101-fix-pvc-logging.3 because the annotations in the pull request description changed
(with .werft/ from main)

if (SnapshotContext.is(workspace.context) || WithPrebuild.is(workspace.context)) {
volumeSnapshotId = workspace.context.snapshotBucketId;
}
const volumeSnapshotId =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changing this to a constant, in hopes that we'll later be able to see the value here. Presently, we do not see the field at all in GCP logs. 🤔

Copy link
Member

Choose a reason for hiding this comment

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

This change has no effect at all. Just removes one of the rare comments.

As commented internally the value is undefined, thus the property gets erased during serialization of the log item's payload, which is basically this:

> JSON.stringify({foo: undefined})
'{}'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @AlexTugarev !

We know lastValidWorkspaceInstanceId has a value (because it was logged in a separate field).

So, now that we know fields with undefined values get removed from the logged object, we'll have to work to determine why workspace.context.snapshotBucketId is sometimes undefined.

@kylos101 kylos101 marked this pull request as ready for review August 11, 2022 04:00
@kylos101
Copy link
Contributor Author

FYI, the preview environment will not work until #11966 is merged and this is rebased.

@kylos101
Copy link
Contributor Author

/hold rebased, will test and remove hold once done

@kylos101
Copy link
Contributor Author

Works:
image

@kylos101
Copy link
Contributor Author

/unhold

@laushinka laushinka self-assigned this Aug 12, 2022
@roboquat roboquat merged commit 32ef4d5 into main Aug 12, 2022
@roboquat roboquat deleted the kylos101/fix-pvc-logging branch August 12, 2022 06:48
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Aug 12, 2022
@kylos101
Copy link
Contributor Author

Dang, still not fixed.

Logs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note-none size/M team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants