-
Notifications
You must be signed in to change notification settings - Fork 3
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
Observe daily desktop users count #311
base: main
Are you sure you want to change the base?
Conversation
|
||
export const observeDesktopUsers = async (pgPoolStats, influxQueryApi) => { | ||
const rows = await influxQueryApi.collectRows(` | ||
from(bucket: "station-machines") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there also a way for us to test this query?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, I think only way to test the query itself would be to spin up InfluxDB instance and execute the query against it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question! We don't have integration tests for Influx queries in this repo yet. Testing wiz @bajtos, wdyt?
CREATE TABLE daily_desktop_users ( | ||
day DATE NOT NULL, | ||
platform TEXT NOT NULL, | ||
user_count INT NOT NULL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
INT has a max value of 2,147,483,647. I think that should be fine for desktop user count for the foreseeable future
export const observeDesktopUsers = async (pgPoolStats, influxQueryApi) => { | ||
const rows = await influxQueryApi.collectRows(` | ||
from(bucket: "station-machines") | ||
|> range(start: -24h) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any concerns about this query being defined as over the last 24h? This means depending on when this function is run, you will get a different count for the day. It would be safer to query the full 24h from the last day, always, for reproducible results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I think it would make sense to query results for previous day with exact start and end time.
|
||
export const observeDesktopUsers = async (pgPoolStats, influxQueryApi) => { | ||
const rows = await influxQueryApi.collectRows(` | ||
from(bucket: "station-machines") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question! We don't have integration tests for Influx queries in this repo yet. Testing wiz @bajtos, wdyt?
Adds loop that observes daily desktop user count and saves it to the database.
Closes CheckerNetwork/roadmap#207