Skip to content

Commit

Permalink
Use browsertime runTime for Graphite/Influx and Grafana anootations a…
Browse files Browse the repository at this point in the history
…nd data.

The old implementation always used the start time for all metrics sent except
browsertime.run metrics (data for each run).

This fix instead makes sure that if metrics (and annotations) uses the browsertime.pageSummary runTime (when the actual first iteration happen).

This makes more sense if you test multiple pages within the same test.
  • Loading branch information
soulgalore committed Jul 6, 2023
1 parent a08a527 commit cc131d1
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 6 deletions.
1 change: 1 addition & 0 deletions lib/plugins/browsertime/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,7 @@ export default class BrowsertimePlugin extends SitespeedioPlugin {

result[resultIndex].statistics.errors = summarizeStats(errorStats);

console.log(result.timestamp);
// Post the result on the queue so other plugins can use it
super.sendMessage('browsertime.pageSummary', result[resultIndex], {
url,
Expand Down
10 changes: 9 additions & 1 deletion lib/plugins/grafana/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import dayjs from 'dayjs';

import { SitespeedioPlugin } from '@sitespeed.io/plugin';

import { send } from './send-annotation.js';
Expand Down Expand Up @@ -90,13 +92,19 @@ export default class GrafanaPlugin extends SitespeedioPlugin {
this.alias[message.url]
);
this.receivedTypesThatFireAnnotations[message.url] = 0;

const timestamp =
message.type === 'browsertime.pageSummary'
? dayjs(message.runTime)
: this.timestamp;

return send(
message.url,
message.group,
absolutePagePath,
this.useScreenshots,
this.screenshotType,
this.timestamp,
timestamp,
this.tsdbType,
this.alias,
this.wptExtras[message.url],
Expand Down
19 changes: 15 additions & 4 deletions lib/plugins/graphite/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,16 @@ export default class GraphitePlugin extends SitespeedioPlugin {
// TODO Here we could add logic to either create a new timestamp or
// use the one that we have for that run. Now just use the one for the
// run
let timestamp = this.timestamp;
if (
message.type === 'browsertime.run' ||
message.type === 'browsertime.pageSummary'
) {
timestamp = dayjs(message.runTime);
}
const dataPoints = this.dataGenerator.dataFromMessage(
message,
message.type === 'browsertime.run'
? dayjs(message.runTime)
: this.timestamp,
timestamp,
this.alias
);

Expand All @@ -176,13 +181,19 @@ export default class GraphitePlugin extends SitespeedioPlugin {
message.url,
this.alias[message.url]
);

const timestamp =
message.type === 'browsertime.pageSummary'
? dayjs(message.runTime)
: this.timestamp;

return send(
message.url,
message.group,
absolutePagePath,
this.useScreenshots,
this.screenshotType,
this.timestamp,
timestamp,
this.alias,
this.wptExtras[message.url],
this.usingBrowsertime,
Expand Down
6 changes: 5 additions & 1 deletion lib/plugins/influxdb/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import isEmpty from 'lodash.isempty';
import intel from 'intel';
import dayjs from 'dayjs';

import { SitespeedioPlugin } from '@sitespeed.io/plugin';
import { InfluxDBSender as Sender } from './sender.js';
import { InfluxDB2Sender as SenderV2 } from './senderV2.js';
Expand Down Expand Up @@ -125,7 +127,9 @@ export default class InfluxDBPlugin extends SitespeedioPlugin {

let data = this.dataGenerator.dataFromMessage(
message,
this.timestamp,
message.type === 'browsertime.pageSummary'
? dayjs(message.runTime)
: this.timestamp,
this.alias
);

Expand Down

0 comments on commit cc131d1

Please sign in to comment.