Skip to content
This repository has been archived by the owner on Jul 12, 2023. It is now read-only.

change chart combos #1886

Merged
merged 12 commits into from
Feb 28, 2021
Merged

change chart combos #1886

merged 12 commits into from
Feb 28, 2021

Conversation

mikehelmick
Copy link
Contributor

Proposed Changes

Update stats visualizations based on user input

  • remove tokens claimed/tokens invalid - that chart has been confusing
  • add tokens claimed and publish requests to make code chart (top of page) -- IF key server metrics are enabled
  • separate out revisions and missing onset from TEKs published (different units)
    • don't show revisions at all (to our knowledge, nobody is using that functionality)
  • add total publish requests and missing onset as overlays on the publish bar chart

image

image

Release Note

* The tokens claimed/invalid chart has been removed (data is still in exports)
* Add total publish requests to the codes/issued claim chart (if key server stats are enabled)
* Separate out revisions and missing onset from TEKs published
* Add total publish requests and missing onset as overlays on the publish bar chart

@google-cla google-cla bot added the cla: yes Auto: added by CLA bot when all committers have signed a CLA. label Feb 28, 2021
@googlebot googlebot added the cla: yes Auto: added by CLA bot when all committers have signed a CLA. label Feb 28, 2021
sethvargo
sethvargo reviewed Feb 28, 2021
View reviewed changes
assets/server/realmadmin/_stats_codes.html Outdated Show resolved Hide resolved
assets/server/realmadmin/_stats_codes.html Outdated Show resolved Hide resolved
pkg/controller/stats/composite.go Outdated Show resolved Hide resolved
sethvargo
sethvargo reviewed Feb 28, 2021
View reviewed changes
Copy link
Member

@sethvargo sethvargo left a comment

Choose a reason for hiding this comment

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

It might be a stupid question, but why don't we just include this information in the existing realm stats. Sure, it would be zeroed if key server stats weren't enabled, but I think that's okay?

assets/server/realmadmin/_stats_codes.html Outdated Show resolved Hide resolved
internal/routes/server.go Outdated Show resolved Hide resolved
internal/routes/server.go Outdated Show resolved Hide resolved
pkg/controller/stats/composite.go Outdated Show resolved Hide resolved
pkg/controller/stats/composite.go Outdated Show resolved Hide resolved
pkg/database/key_server_stats.go Outdated Show resolved Hide resolved
pkg/database/realm_stats.go Show resolved Hide resolved
pkg/database/key_server_stats.go Show resolved Hide resolved
mikehelmick
mikehelmick commented Feb 28, 2021
View reviewed changes
pkg/database/key_server_stats.go
Error
}

// ListKeyServerStats retrieves the key-server statistics configuration for all realms
func (db *Database) ListKeyServerStats() ([]*KeyServerStats, error) {
var stats []*KeyServerStats
if err := db.db.
Model(&KeyServerStatsDay{}).
Model(&KeyServerStats{}).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was a bug

sethvargo
sethvargo reviewed Feb 28, 2021
View reviewed changes
pkg/database/database.go Outdated Show resolved Hide resolved
pkg/database/key_server_stats.go Outdated Show resolved Hide resolved
pkg/database/key_server_stats.go Outdated Show resolved Hide resolved
sethvargo
sethvargo approved these changes Feb 28, 2021
View reviewed changes
Copy link
Member

@sethvargo sethvargo left a comment

Choose a reason for hiding this comment

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

/lgtm

@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mikehelmick, sethvargo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [mikehelmick,sethvargo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-robot google-oss-robot merged commit 919f7d6 into google:main Feb 28, 2021
@mikehelmick mikehelmick deleted the combineCharts branch March 1, 2021 00:26
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sethvargo sethvargo sethvargo approved these changes

@whaught whaught Awaiting requested review from whaught

@yegle yegle Awaiting requested review from yegle

Assignees

@sethvargo sethvargo

Labels
cla: yes Auto: added by CLA bot when all committers have signed a CLA. kind/statistics
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mikehelmick @google-oss-robot @sethvargo @googlebot