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

IDE metrics server #11542

Merged
merged 9 commits into from
Aug 5, 2022
Merged

IDE metrics server #11542

merged 9 commits into from
Aug 5, 2022

Conversation

iQQBot
Copy link
Contributor

@iQQBot iQQBot commented Jul 21, 2022

Description

This PR introduce a new component ide-metrics, which is a public endpoint, which can allow some short live client report metrics and error log (not in this PR)

Related Issue(s)

Fix #11134

How to test

For test convenience I wrote some test clients, and they will be deleted before merge

  1. open this branch in gitpod.io
  2. switch to /workspace/gitpod/components/ide-metrics
  3. run go run main.go test-client, you will see send success message
  4. use ./dev/preview/portforward-monitoring-satellite.sh -c harvester to open promethues
  5. you will see the metrics gitpod_supervisor_frontend_error_total is increase (There is an interval for scraping, you can refresh in 1min)

image

another example:

  1. open workspace in preview environment
  2. open chrome dev tool -> Console
  3. run command throw new Error("aa"), them switch to Network tab, you will see AddCounter request
  4. monitor promethues metrics gitpod_supervisor_frontend_error_total should increase
    image

Release Notes

NONE

Documentation

Werft options:

  • /werft with-preview
  • /werft publish-to-npm
  • /werft withObservabilityBranch=pd/ide-metrics

gitpod-ws.code-workspace Outdated Show resolved Hide resolved
components/ide-metrics-api/metrics.proto Outdated Show resolved Hide resolved
components/ide-metrics-api/metrics.proto Outdated Show resolved Hide resolved
components/ide-metrics-api/metrics.proto Outdated Show resolved Hide resolved
@socket-security
Copy link

socket-security bot commented Jul 31, 2022

Socket Security Report

👍 No new dependency issues detected in pull request

Socket.dev scan summary
Issue Status
Did you mean? ✅ no new possible package typos
Install scripts ✅ no new install scripts
Telemetry ✅ no new telemetry
Troll package ✅ no new troll packages
Malware ✅ no new malware
Native code ✅ no new native modules

Powered by socket.dev

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-pd-ide-metrics.10 because the annotations in the pull request description changed
(with .werft/ from main)

@iQQBot iQQBot force-pushed the pd/ide-metrics branch 10 times, most recently from 5c9e639 to fa6a57e Compare August 3, 2022 01:33
@gitguardian
Copy link

gitguardian bot commented Aug 3, 2022

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
Once a secret has been leaked into a git repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

@iQQBot iQQBot changed the title [WIP] ide metrics server ide metrics server Aug 3, 2022
@iQQBot iQQBot changed the title ide metrics server IDE metrics server Aug 3, 2022
@iQQBot iQQBot force-pushed the pd/ide-metrics branch 3 times, most recently from 9994d75 to 0183d20 Compare August 3, 2022 07:29
@iQQBot
Copy link
Contributor Author

iQQBot commented Aug 5, 2022

@iQQBot it looks like that's awaiting the @gitpod-io/platform team's approval

Yes, so I put a hold label here, wait https://github.com/gitpod-io/ops/pull/3666 get merged first. Thanks 🙏

@mustard-mh
Copy link
Contributor

I tested manually throw new Error('hello') in DevTool Terminal and it worked well, but errors like the image below seem not been caught?

image

@akosyakov
Copy link
Member

I tested manually throw new Error('hello') in DevTool Terminal and it worked well, but errors like the image below seem not been caught?

@mustard-mh It is alright. The goal of this PR is to bring IDE metrics component to production. We can work on concrete metrics later.

@akosyakov
Copy link
Member

@iQQBot let's close #11134 with it

I will create another issue for GCP error reporting. What is left here only rate limitting?

@iQQBot
Copy link
Contributor Author

iQQBot commented Aug 5, 2022

@iQQBot let's close #11134 with it

I will create another issue for GCP error reporting. What is left here only rate limitting?

Yes, but I am not sure what do we need to base the rate limit on, IP address?

@akosyakov
Copy link
Member

akosyakov commented Aug 5, 2022

Yes, but I am not sure what do we need to base the rate limit on, IP address?

Could you check how we do it in ws-manager please? 🙏 It can be another PR, let's create an issue for it for now. We need to land this PR to unblock #11910

@akosyakov akosyakov mentioned this pull request Aug 5, 2022
@iQQBot
Copy link
Contributor Author

iQQBot commented Aug 5, 2022

Could you check how we do it in ws-manager please? 🙏 It can be another PR, let's create an issue for it for now. We need to land this PR to unblock #11910

ws-manager just rate limit for the method name https://github.com/gitpod-io/ops/blob/main/deploy/workspace/installer-values.yaml#L25-L32 because it client only server and ws-proxy

iQQBot and others added 2 commits August 5, 2022 09:54
@akosyakov
Copy link
Member

akosyakov commented Aug 5, 2022

@iQQBot We can do the same or on level of metric names? but let's do it later, we don't have anyone calling it yet so often, anyway

@iQQBot
Copy link
Contributor Author

iQQBot commented Aug 5, 2022

@iQQBot We can do the same or on level of metric names? but let's do it later, we don't have anyone calling it yet so often, anyway

Let's see how it looks in practice and then see how we can adjust ratelimit

@iQQBot
Copy link
Contributor Author

iQQBot commented Aug 5, 2022

/unhold

@roboquat roboquat merged commit 3e7f3f5 into main Aug 5, 2022
@roboquat roboquat deleted the pd/ide-metrics branch August 5, 2022 10:52
@iQQBot iQQBot removed team: webapp Issue belongs to the WebApp team team: workspace Issue belongs to the Workspace team team: delivery Issue belongs to the self-hosted team labels Aug 5, 2022
@roboquat roboquat added deployed: IDE IDE change is running in production deployed Change is completely running in production labels Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: IDE IDE change is running in production deployed Change is completely running in production release-note-none size/XXL team: IDE
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

IDE clients observability of metrics
7 participants