Skip to content

feat: grafana dashboard configuration#184

Open
bornast wants to merge 21 commits intomasterfrom
feat/grafana-dashboards
Open

feat: grafana dashboard configuration#184
bornast wants to merge 21 commits intomasterfrom
feat/grafana-dashboards

Conversation

@bornast
Copy link
Copy Markdown
Member

@bornast bornast commented Mar 18, 2026

This PR makes dashboard configuration more flexible. It enables passing different dashboards to the Grafana builder component, adds support for custom dashboard builders, and allows adding custom panels to the existing web server SLO dashboard builder.

@bornast bornast requested review from droguljic and mandryllo March 18, 2026 16:40
@bornast bornast added the Don't merge Do not merge this PR label Mar 18, 2026
h: number;
};
}
export type PanelPosition = {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why break this type out of the Panel namespace?

// - panels (long-window SLI, long-window error budget)
// - alerts (long-window burn, short-window burn)
export namespace GrafanaDashboard {
export type DataSources = {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would opt for singular since this is not an array type.

prometheus?: pulumi.Output<string>;
};

export interface DashboardConfig {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not just Config or Configuration?

title: pulumi.Output<string>;
panels: Grafana.Panel[] = [];
tags?: pulumi.Output<string[]>;
private panelBuilders: ((
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This inline type is not that readable; extract it into a dedicated type variable.

name: string;
grafanaIamRole: aws.iam.Role;
prometheusDataSource?: grafana.oss.DataSource;
dashboards: grafana.oss.Dashboard[] = [];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why initialize it as an empty array and not mark it as optional?

Base automatically changed from feat/grafana-comp to master March 26, 2026 14:32
@droguljic droguljic added this to the v2 milestone Mar 26, 2026
Comment on lines +32 to +33
timezone: 'browser',
refresh: '10s',
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I feel these should be configurable with these values as defaults

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants