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.

Sign up for GitHub

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

Jump to bottom

telemetry: Enable data fields that were disabled #8915

Merged
merged 1 commit into from
Mar 29, 2022
Merged

telemetry: Enable data fields that were disabled #8915

merged 1 commit into from
Mar 29, 2022

Conversation

Pothulapati
Copy link
Contributor

@Pothulapati Pothulapati commented Mar 21, 2022 •
edited
Loading

Description

#8617 disabled the telemetry
fields as those database queries were causing heavy CPU performance.

#8858 fixed that issue by
adding indexes for those queries.

This PR also adds tracing for the telemetry data provider so that we
can also count how long they take usually, and how frequently these
are called in prod.

Signed-off-by: Tarun Pothulapati tarun@gitpod.io

Related Issue(s)

Fixes #

How to test

Release Notes

[telemetry] enable data fields that were previously disabled

Documentation

@Pothulapati
Copy link
Contributor Author

Pothulapati commented Mar 21, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-helm=true with-observability=true

👍 started the job as gitpod-build-tar-tracing-telemetry.3

@Pothulapati Pothulapati changed the title telemetry: Enable more fields along with tracing telemetry: Enable data fields Mar 21, 2022
@Pothulapati Pothulapati force-pushed the tar/tracing-telemetry branch from 1b10349 to 2bccd6d Compare March 21, 2022 15:08
@Pothulapati Pothulapati changed the title telemetry: Enable data fields telemetry: Enable data fields that were disabled Mar 21, 2022
@Pothulapati Pothulapati force-pushed the tar/tracing-telemetry branch from 2bccd6d to 273cbf7 Compare March 21, 2022 17:01
@roboquat roboquat added size/S and removed size/XS labels Mar 21, 2022
@Pothulapati
Copy link
Contributor Author

Pothulapati commented Mar 21, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-helm=true with-observability=true

👍 started the job as gitpod-build-tar-tracing-telemetry.6

@kylos101
Copy link
Contributor

kylos101 commented Mar 21, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-helm=true with-observability=true with-clean-slate-deployment=true

👍 started the job as gitpod-build-tar-tracing-telemetry.7

@Pothulapati Pothulapati force-pushed the tar/tracing-telemetry branch from 273cbf7 to 91f9fd2 Compare March 21, 2022 17:57
@Pothulapati
Copy link
Contributor Author

Pothulapati commented Mar 22, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-helm=true with-observability=true with-clean-slate-deployment=true

👍 started the job as gitpod-build-tar-tracing-telemetry.9

@Pothulapati Pothulapati requested a review from a team March 22, 2022 06:35
@github-actions github-actions bot added the team: delivery Issue belongs to the self-hosted team label Mar 22, 2022
@Pothulapati Pothulapati marked this pull request as ready for review March 22, 2022 06:37
@Pothulapati Pothulapati requested a review from a team March 22, 2022 06:37
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Mar 22, 2022
geropl
geropl requested changes Mar 22, 2022
View reviewed changes
Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

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

THx for the PR!

Could you add the try...finally? Happy to quickly approve afterwards. 👍

@Pothulapati Pothulapati force-pushed the tar/tracing-telemetry branch from 91f9fd2 to 4a6854c Compare March 22, 2022 10:42
@roboquat roboquat added size/M and removed size/S labels Mar 22, 2022
@Pothulapati Pothulapati requested a review from geropl March 22, 2022 10:42
geropl
geropl previously approved these changes Mar 22, 2022
View reviewed changes
Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

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

Thanks!

@Pothulapati
Copy link
Contributor Author

Pothulapati commented Mar 23, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-helm=true with-observability=true with-clean-slate-deployment=true

👍 started the job as gitpod-build-tar-tracing-telemetry.11

@Pothulapati
Copy link
Contributor Author

Pothulapati commented Mar 23, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-clean-slate-deployment=true

👍 started the job as gitpod-build-tar-tracing-telemetry.12

@Pothulapati Pothulapati removed the request for review from corneliusludmann March 23, 2022 06:35
@iQQBot
Copy link
Contributor

iQQBot commented Mar 23, 2022

/hold
this PR is stuck tide

@iQQBot
Copy link
Contributor

iQQBot commented Mar 23, 2022

@Pothulapati maybe you need rebase to change commit SHA

easyCZ
easyCZ previously approved these changes Mar 28, 2022
View reviewed changes
Copy link
Member

@easyCZ easyCZ left a comment

Choose a reason for hiding this comment

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

LGTM

mrsimonemms
mrsimonemms previously approved these changes Mar 28, 2022
View reviewed changes
@Pothulapati
telemetry: Enable data fields that were disabled
eb9c8ac 
#8617 disabled the telemetry
fields as those database queries were causing heavy CPU performance.

#8858 fixed that issue by
adding indexes for those queries.

This PR also adds tracing for the telemetry data provider so that we
can also count how long they take usually, and how frequently these
are called.

Signed-off-by: Tarun Pothulapati <tarun@gitpod.io>
@Pothulapati Pothulapati dismissed stale reviews from mrsimonemms, easyCZ, and geropl via eb9c8ac March 28, 2022 14:31
@Pothulapati Pothulapati force-pushed the tar/tracing-telemetry branch from 4a6854c to eb9c8ac Compare March 28, 2022 14:31
@mrsimonemms mrsimonemms self-requested a review March 28, 2022 14:32
@Pothulapati Pothulapati requested review from easyCZ and geropl March 28, 2022 14:33
@Pothulapati
Copy link
Contributor Author

/unhold

@csweichel
Copy link
Contributor

@geropl or @easyCZ: one of you needs to approve again

@roboquat roboquat merged commit 2f86163 into main Mar 29, 2022
@roboquat roboquat deleted the tar/tracing-telemetry branch March 29, 2022 11:50
@roboquat roboquat added the deployed: webapp Meta team change is running in production label Mar 31, 2022
@Pothulapati Pothulapati mentioned this pull request Apr 4, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrsimonemms mrsimonemms mrsimonemms approved these changes

@easyCZ easyCZ easyCZ approved these changes

@geropl geropl Awaiting requested review from geropl

Assignees
No one assigned
Labels
deployed: webapp Meta team change is running in production release-note size/M team: delivery Issue belongs to the self-hosted team team: webapp Issue belongs to the WebApp team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@Pothulapati @kylos101 @iQQBot @csweichel @mrsimonemms @easyCZ @geropl @roboquat