-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
DNM: Size issues e2e demo #104698
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
DNM: Size issues e2e demo #104698
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| from collections.abc import Iterator | ||
| from datetime import datetime, timezone | ||
| from typing import Any | ||
| from uuid import uuid4 | ||
|
|
||
| from sentry.issues.grouptype import PreprodDeltaGroupType | ||
| from sentry.issues.issue_occurrence import IssueOccurrence | ||
|
|
||
|
|
||
| class SizeRegressionOccurrenceBuilder: | ||
| def __init__(self): | ||
| self.issue_title = None | ||
| self.issue_subtitle = None | ||
| self.project_id = None | ||
|
|
||
| def build(self) -> tuple[IssueOccurrence, dict[str, Any]]: | ||
| id = uuid4() | ||
| assert self.issue_title is not None, "issue_title must be set" | ||
| assert self.issue_title is not None, "issue_subtitle must be set" | ||
| assert self.project_id is not None, "project_id must be set" | ||
|
|
||
| current_timestamp = datetime.now(timezone.utc) | ||
| id = uuid4().hex | ||
| event_id = uuid4().hex | ||
|
|
||
| event_data = { | ||
| "event_id": event_id, | ||
| "platform": "other", | ||
| "project_id": self.project_id, | ||
| "received": current_timestamp.isoformat(), | ||
| "sdk": None, | ||
| "tags": {}, | ||
| "timestamp": current_timestamp.isoformat(), | ||
| "environment": "prod", | ||
| } | ||
|
|
||
| return ( | ||
| IssueOccurrence( | ||
| id=id, | ||
| event_id=event_id, | ||
| issue_title=self.issue_title, | ||
| subtitle=self.issue_subtitle, | ||
| project_id=self.project_id, | ||
| fingerprint=uuid4().hex, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Fingerprint passed as string instead of listThe |
||
| type=PreprodDeltaGroupType, | ||
| detection_time=current_timestamp, | ||
| level="error", | ||
| resource_id="", | ||
| evidence_data={ | ||
| "head_install_size_bytes": self.head_install_size_bytes, | ||
| "base_install_size_bytes": self.base_install_size_bytes, | ||
| "head_artifact_id": self.head_artifact_id, | ||
| "base_artifact_id": self.base_artifact_id, | ||
| }, | ||
| evidence_display={}, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Evidence display passed as dict instead of listThe |
||
| culprit="", | ||
| ), | ||
| event_data, | ||
| ) | ||
|
|
||
|
|
||
| # from sentry.issues.grouptype import PreprodStaticGroupType | ||
| # class PreprodStaticIssueOccurrenceBuilder: | ||
| # def __init__(self): | ||
| # self.issue_title = None | ||
| # self.project_id = None | ||
| # | ||
| # def build(self) -> tuple[IssueOccurrence, dict[str, Any]]: | ||
| # id = uuid4() | ||
| # assert self.issue_title is not None, "issue_title must be set" | ||
| # assert self.project_id is not None, "project_id must be set" | ||
| # | ||
| # current_timestamp = datetime.now(timezone.utc) | ||
| # id = uuid4().hex | ||
| # event_id = uuid4().hex | ||
| # | ||
| # event_data = { | ||
| # "event_id": event_id, | ||
| # "platform": "other", | ||
| # "project_id": self.project_id, | ||
| # "received": current_timestamp.isoformat(), | ||
| # "sdk": None, | ||
| # "tags": {}, | ||
| # "timestamp": current_timestamp.isoformat(), | ||
| # # "contexts": {"monitor": get_monitor_environment_context(monitor_env)}, | ||
| # "environment": "prod", | ||
| # # "fingerprint": [incident.grouphash], | ||
| # } | ||
| # | ||
| # return ( | ||
| # IssueOccurrence( | ||
| # id=id, | ||
| # event_id=event_id, | ||
| # issue_title=self.issue_title, | ||
| # subtitle="", | ||
| # project_id=self.project_id, | ||
| # # TODO: fix | ||
| # fingerprint=uuid4().hex, | ||
| # type=PreprodStaticIssueOccurrenceBuilder, | ||
| # # Now? | ||
| # detection_time=current_timestamp, | ||
| # level="error", | ||
| # resource_id="", | ||
| # evidence_data={}, | ||
| # evidence_display={}, | ||
| # culprit="", | ||
| # ), | ||
| # event_data, | ||
| # ) | ||
| # def insight_to_occurrences(name: str, insight: dict[str, any]) -> list[SizeIssueOccurrenceBuilder]: | ||
| # builder = SizeIssueOccurrenceBuilder() | ||
| # builder.issue_title = f"Bad size {name}" | ||
| # return [builder] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| from django.db import router, transaction | ||
| from django.utils import timezone | ||
|
|
||
| from sentry.issues.producer import PayloadType, produce_occurrence_to_kafka | ||
| from sentry.models.files.file import File | ||
| from sentry.preprod.models import ( | ||
| PreprodArtifact, | ||
|
|
@@ -22,6 +23,8 @@ | |
| from sentry.utils import metrics | ||
| from sentry.utils.json import dumps_htmlsafe | ||
|
|
||
| from .issues import SizeRegressionOccurrenceBuilder | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
|
|
@@ -477,6 +480,39 @@ def _run_size_analysis_comparison( | |
| comparison.state = PreprodArtifactSizeComparison.State.SUCCESS | ||
| comparison.save() | ||
|
|
||
| ################################################################ | ||
|
|
||
| head_project = head_size_metric.preprod_artifact.project | ||
| threshold = head_project.get_option("sentry:preprod_size_issues_delta_install_threshhold_kb") | ||
| diff_item = comparison_results.size_metric_diff_item | ||
| actual = diff_item.head_install_size - diff_item.base_install_size | ||
|
|
||
| logger.warning( | ||
| "!!!!!!!!!!!!!!!!? issue threshold: %d actual: %d base: %d head: %d", | ||
| threshold, | ||
| actual, | ||
| diff_item.base_install_size, | ||
| diff_item.head_install_size, | ||
| ) | ||
| if actual >= threshold * 1024: | ||
| logger.warning("!!!! trigger issue") | ||
| builder = SizeRegressionOccurrenceBuilder() | ||
| builder.project_id = head_project.id | ||
| builder.issue_title = f"Install size regression" | ||
| builder.issue_subtitle = f"Size increased {actual // 1024}kb" | ||
| builder.head_install_size_bytes = diff_item.head_install_size | ||
| builder.base_install_size_bytes = diff_item.base_install_size | ||
| builder.base_artifact_id = base_size_metric.preprod_artifact.id | ||
| builder.head_artifact_id = head_size_metric.preprod_artifact.id | ||
| occurrence, event_data = builder.build() | ||
| produce_occurrence_to_kafka( | ||
| payload_type=PayloadType.OCCURRENCE, | ||
| occurrence=occurrence, | ||
| event_data=event_data, | ||
| ) | ||
| logger.warning("!!!! done") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Debug logging statements accidentally committed to production codeSeveral |
||
| ################################################################ | ||
|
|
||
| logger.info( | ||
| "preprod.size_analysis.compare.success", | ||
| extra={"comparison_id": comparison.id}, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| import {Fragment} from 'react'; | ||
|
|
||
| import TextBlock from 'sentry/views/settings/components/text/textBlock'; | ||
| import Feature from 'sentry/components/acl/feature'; | ||
| import {ButtonBar} from 'sentry/components/core/button/buttonBar'; | ||
| import FeedbackButton from 'sentry/components/feedbackButton/feedbackButton'; | ||
| import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle'; | ||
| import {t} from 'sentry/locale'; | ||
| import ProjectsStore from 'sentry/stores/projectsStore'; | ||
| import SettingsPageHeader from 'sentry/views/settings/components/settingsPageHeader'; | ||
| import {useProjectSettingsOutlet} from 'sentry/views/settings/project/projectSettingsLayout'; | ||
| import Access from 'sentry/components/acl/access'; | ||
| import {ProjectPermissionAlert} from 'sentry/views/settings/project/projectPermissionAlert'; | ||
| import Form from 'sentry/components/forms/form'; | ||
| import JsonForm from 'sentry/components/forms/jsonForm'; | ||
| import useOrganization from 'sentry/utils/useOrganization'; | ||
| import type {JsonFormObject} from 'sentry/components/forms/types'; | ||
|
|
||
| const scopeForEdit = 'project:write'; | ||
|
|
||
|
|
||
| //const defaultInstallSizeAbsoluteDeltaIssueThresholdKb = 500; | ||
|
|
||
| export default function PreprodSettings() { | ||
| const organization = useOrganization(); | ||
| const {project} = useProjectSettingsOutlet(); | ||
|
|
||
| const formGroups: JsonFormObject[] = [ | ||
| { | ||
| title: t('Size Analysis Issues'), | ||
| fields: [ | ||
| { | ||
| name: 'sentry:preprod_size_issues_is_16kb_ready', | ||
| type: 'boolean', | ||
| label: t('Create 16kb ready issues'), | ||
| help: t('Toggles whether or not to create issues for 16kb readiness.'), | ||
| getData: data => ({options: data}), | ||
| }, | ||
| { | ||
| name: 'sentry:preprod_size_issues_delta_install_threshhold_kb', | ||
| type: 'number', | ||
| label: t('Install size regression threshold (kb)'), | ||
| help: t('If there is an regression in install size larger than the threshold an issue will be created.'), | ||
| getData: data => ({options: data}), | ||
| }, | ||
| ], | ||
| }, | ||
| ]; | ||
|
|
||
| return ( | ||
| <Fragment> | ||
| <Feature features="organizations:preprod-issues" renderDisabled> | ||
| <SentryDocumentTitle title={t('Preprod')} /> | ||
| <SettingsPageHeader | ||
| title={t('Preprod')} | ||
| action={ | ||
| <ButtonBar gap="lg"> | ||
| <FeedbackButton /> | ||
| </ButtonBar> | ||
| } | ||
| /> | ||
| <Access access={[scopeForEdit]} project={project}> | ||
| {({hasAccess}) => ( | ||
| <Fragment> | ||
| <TextBlock> | ||
| {t(` | ||
| Configure size analysis and build distribution. | ||
| `)} | ||
| </TextBlock> | ||
|
|
||
| <ProjectPermissionAlert access={[scopeForEdit]} project={project} /> | ||
|
|
||
| <Form | ||
| saveOnBlur | ||
| apiMethod="PUT" | ||
| apiEndpoint={`/projects/${organization.slug}/${project.slug}/`} | ||
| initialData={project.options} | ||
| onSubmitSuccess={(response) => ProjectsStore.onUpdateSuccess(response)} | ||
| > | ||
| <JsonForm | ||
| disabled={!hasAccess} | ||
| features={new Set(organization.features)} | ||
| forms={formGroups} | ||
| /> | ||
| </Form> | ||
|
|
||
| </Fragment> | ||
| )} | ||
| </Access> | ||
| </Feature> | ||
| </Fragment> | ||
| ); | ||
| } |
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.
Bug: Wrong variable checked in assertion for issue_subtitle
The assertion on line 19 checks
self.issue_title is not Nonebut the error message says "issue_subtitle must be set". This copy-paste error meansissue_subtitleis never validated, so it could beNonewhen passed toIssueOccurrence(subtitle=self.issue_subtitle, ...), potentially causing unexpected behavior or errors downstream.