Skip to content

feat(dam): Expose derived metrics in dashboards#32798

Merged
shruthilayaj merged 13 commits into
masterfrom
feat/derived-metrics
Apr 5, 2022
Merged

feat(dam): Expose derived metrics in dashboards#32798
shruthilayaj merged 13 commits into
masterfrom
feat/derived-metrics

Conversation

@matejminar

@matejminar matejminar commented Mar 21, 2022

Copy link
Copy Markdown
Member

Add support for derived metrics in Dashboards.

Screen Shot 2022-04-04 at 8 42 55 AM

@github-actions

github-actions Bot commented Mar 21, 2022

Copy link
Copy Markdown
Contributor

size-limit report 📦

Path Size
src/sentry/static/sentry/dist/entrypoints/app.js 61.16 KB (+0.03% 🔺)
src/sentry/static/sentry/dist/entrypoints/sentry.css 69.97 KB (0%)

@shruthilayaj shruthilayaj changed the title feat(dam): Add derived metrics feat(dam): Expose derived metrics in dashboards Apr 4, 2022
@shruthilayaj shruthilayaj marked this pull request as ready for review April 4, 2022 12:44
@shruthilayaj shruthilayaj requested review from a team April 4, 2022 12:44
@shruthilayaj shruthilayaj requested a review from a team as a code owner April 4, 2022 12:44
@shruthilayaj shruthilayaj removed request for a team April 4, 2022 12:44

@edwardgou-sentry edwardgou-sentry left a comment

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.

mostly nits!

if (
column.kind === 'field' ||
column.kind === 'equation' ||
column.kind === 'calculatedField'

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.

nit: Should we use FieldValueKind.FIELD and FieldValueKind.EQUATION and add calculatedField to FieldValueKind?

@shruthilayaj shruthilayaj Apr 5, 2022

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.

I'll do this and the comment below in a follow up (it's a bigger change and causes a lot of ts errors)!

@shruthilayaj shruthilayaj Apr 5, 2022

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.

Ok @edwardgou-sentry actually I'm going to make a ticket and kick this can to when we clean up tech debt 😛

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.

Also I don't know that making these enums provides us with much benefit? Enums might just increase the size of the transpiled code?

}
| {
field: string;
kind: 'calculatedField';

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.

nit: maybe not within scope of this pr either but I think we can probably also use the FieldValueKind enum here

Comment thread static/app/views/eventsV2/table/queryField.tsx Outdated
@shruthilayaj shruthilayaj self-assigned this Apr 5, 2022

@edwardgou-sentry edwardgou-sentry left a comment

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.

Looks good to me! The other comments are definitely nits and not blockers

@shruthilayaj shruthilayaj merged commit 75aed4d into master Apr 5, 2022
@shruthilayaj shruthilayaj deleted the feat/derived-metrics branch April 5, 2022 17:56
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants