Skip to content
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

feat(sourcemaps): Implement new tables supporting debug ids #44572

Merged
merged 17 commits into from
Feb 28, 2023

Conversation

iambriccardo
Copy link
Member

@iambriccardo iambriccardo commented Feb 14, 2023

This PR aims at implementing the new tables used to support debug ids for source maps.

class ArtifactBundle(Model):
__include_in_export__ = False

bundle_id = models.CharField(max_length=32, null=True)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
bundle_id = models.CharField(max_length=32, null=True)
bundle_id = models.UUIDField(null=True)

bundle_id = models.CharField(max_length=32, null=True)
organization_id = BoundedBigIntegerField(db_index=True)
# TODO: define max length.
release_name = models.CharField(null=True)
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't this also be an integer field, like dist_id?

If we're going with strings, use the same maxlength as on Release:

version = models.CharField(max_length=DB_VERSION_LENGTH)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! I was searching for the name of the release in the model and didn't think it could have been named version 😅.

Copy link
Member

Choose a reason for hiding this comment

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

We cannot guarantee that a release exists yet. I think since we likely have one artifact per release, a string is fine. Alternatively one could deduplicate through another table but that seems pointless.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will add @beezz to check if the tables are fine.



@region_silo_only_model
class ProjectArtifactBundleIndex(Model):
Copy link
Member

Choose a reason for hiding this comment

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

nit: I would drop the Index part of these intermediate tables, see for example

class ProjectTeam(Model):
__include_in_export__ = True
project = FlexibleForeignKey("sentry.Project")
team = FlexibleForeignKey("sentry.Team")

Copy link
Member Author

Choose a reason for hiding this comment

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

I have followed the FileBlobIndex pattern but I agree, it's not an index. It's its own table.

@iambriccardo
Copy link
Member Author

@mitsuhiko we were discussing with @beezz about trying to use a hash index on the debug_id, we are not sure about the performance though, because hash indexes on Postgres seem to be implemented with buckets that will require linear scan inside. On the Postgres website it is specified that the planner would try to use them for equality comparisons which is what we want for debug_id so maybe it may be worth trying.

Copy link
Contributor

@beezz beezz left a comment

Choose a reason for hiding this comment

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

Please add the tables to the database router before merging so they are created on the files cluster since we have FK to the sentry.File table.

("organization_id", "bundle_id"),
("organization_id", "release_name"),
)
index_together = (("organization_id", "release_name"),)
Copy link
Contributor

Choose a reason for hiding this comment

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

You can remove this index since this set of columns is already in unique_together, which creates an index implicitly to enforce the uniqueness.

class DebugIdArtifactBundle(Model):
__include_in_export__ = False

debug_id = models.UUIDField(unique=True, db_index=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a requirement for having this as a separate uuid field? Is the implicit primary key id field not enough?

@iambriccardo iambriccardo changed the title feat(source-maps): Implement new tables feat(source-maps): Implement new tables supporting debug ids Feb 20, 2023
@iambriccardo iambriccardo changed the title feat(source-maps): Implement new tables supporting debug ids feat(sourcemaps): Implement new tables supporting debug ids Feb 20, 2023
@github-actions
Copy link
Contributor

This PR has a migration; here is the generated SQL for src/sentry/migrations/0361_debug_id_artifact_bundle.py ()

--
-- Create model ArtifactBundle
--
CREATE TABLE "sentry_artifactbundle" ("id" bigserial NOT NULL PRIMARY KEY, "organization_id" bigint NOT NULL, "bundle_id" uuid NULL, "artifact_count" integer NOT NULL CHECK ("artifact_count" >= 0), "file_id" bigint NOT NULL);
ALTER TABLE "sentry_artifactbundle" ADD CONSTRAINT "sentry_artifactbundle_file_id_1d6752c9_fk_sentry_file_id" FOREIGN KEY ("file_id") REFERENCES "sentry_file" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "sentry_artifactbundle" VALIDATE CONSTRAINT "sentry_artifactbundle_file_id_1d6752c9_fk_sentry_file_id";
CREATE UNIQUE INDEX CONCURRENTLY "sentry_artifactbundle_organization_id_bundle_id_19f21a4b_uniq" ON "sentry_artifactbundle" ("organization_id", "bundle_id");
ALTER TABLE "sentry_artifactbundle" ADD CONSTRAINT "sentry_artifactbundle_organization_id_bundle_id_19f21a4b_uniq" UNIQUE USING INDEX "sentry_artifactbundle_organization_id_bundle_id_19f21a4b_uniq";
CREATE INDEX CONCURRENTLY "sentry_artifactbundle_organization_id_b5a13770" ON "sentry_artifactbundle" ("organization_id");
CREATE INDEX CONCURRENTLY "sentry_artifactbundle_file_id_1d6752c9" ON "sentry_artifactbundle" ("file_id");
--
-- Create model ReleaseArtifactBundle
--
CREATE TABLE "sentry_releaseartifactbundle" ("id" bigserial NOT NULL PRIMARY KEY, "organization_id" bigint NOT NULL, "release_name" varchar(250) NOT NULL, "dist_id" bigint NULL, "artifact_bundle_id" bigint NOT NULL);
ALTER TABLE "sentry_releaseartifactbundle" ADD CONSTRAINT "sentry_releaseartifa_artifact_bundle_id_e0298a12_fk_sentry_ar" FOREIGN KEY ("artifact_bundle_id") REFERENCES "sentry_artifactbundle" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "sentry_releaseartifactbundle" VALIDATE CONSTRAINT "sentry_releaseartifa_artifact_bundle_id_e0298a12_fk_sentry_ar";
CREATE UNIQUE INDEX CONCURRENTLY "sentry_releaseartifactbu_organization_id_release__ebde7736_uniq" ON "sentry_releaseartifactbundle" ("organization_id", "release_name", "dist_id", "artifact_bundle_id");
ALTER TABLE "sentry_releaseartifactbundle" ADD CONSTRAINT "sentry_releaseartifactbu_organization_id_release__ebde7736_uniq" UNIQUE USING INDEX "sentry_releaseartifactbu_organization_id_release__ebde7736_uniq";
CREATE INDEX CONCURRENTLY "sentry_releaseartifactbundle_organization_id_10fc7929" ON "sentry_releaseartifactbundle" ("organization_id");
CREATE INDEX CONCURRENTLY "sentry_releaseartifactbundle_artifact_bundle_id_e0298a12" ON "sentry_releaseartifactbundle" ("artifact_bundle_id");
--
-- Create model ProjectArtifactBundle
--
CREATE TABLE "sentry_projectartifactbundle" ("id" bigserial NOT NULL PRIMARY KEY, "project_id" bigint NOT NULL, "artifact_bundle_id" bigint NOT NULL);
ALTER TABLE "sentry_projectartifactbundle" ADD CONSTRAINT "sentry_projectartifa_artifact_bundle_id_c3f68015_fk_sentry_ar" FOREIGN KEY ("artifact_bundle_id") REFERENCES "sentry_artifactbundle" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "sentry_projectartifactbundle" VALIDATE CONSTRAINT "sentry_projectartifa_artifact_bundle_id_c3f68015_fk_sentry_ar";
CREATE UNIQUE INDEX CONCURRENTLY "sentry_projectartifactbu_project_id_artifact_bund_b374329e_uniq" ON "sentry_projectartifactbundle" ("project_id", "artifact_bundle_id");
ALTER TABLE "sentry_projectartifactbundle" ADD CONSTRAINT "sentry_projectartifactbu_project_id_artifact_bund_b374329e_uniq" UNIQUE USING INDEX "sentry_projectartifactbu_project_id_artifact_bund_b374329e_uniq";
CREATE INDEX CONCURRENTLY "sentry_projectartifactbundle_project_id_7588084b" ON "sentry_projectartifactbundle" ("project_id");
CREATE INDEX CONCURRENTLY "sentry_projectartifactbundle_artifact_bundle_id_c3f68015" ON "sentry_projectartifactbundle" ("artifact_bundle_id");
--
-- Create model DebugIdArtifactBundle
--
CREATE TABLE "sentry_debugidartifactbundle" ("id" bigserial NOT NULL PRIMARY KEY, "organization_id" bigint NOT NULL, "debug_id" uuid NOT NULL, "source_file_type" integer NOT NULL, "date_added" timestamp with time zone NOT NULL, "date_last_accessed" timestamp with time zone NOT NULL, "artifact_bundle_id" bigint NOT NULL);
ALTER TABLE "sentry_debugidartifactbundle" ADD CONSTRAINT "sentry_debugidartifa_artifact_bundle_id_91f3140f_fk_sentry_ar" FOREIGN KEY ("artifact_bundle_id") REFERENCES "sentry_artifactbundle" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "sentry_debugidartifactbundle" VALIDATE CONSTRAINT "sentry_debugidartifa_artifact_bundle_id_91f3140f_fk_sentry_ar";
CREATE UNIQUE INDEX CONCURRENTLY "sentry_debugidartifactbu_debug_id_artifact_bundle_0e34c4ff_uniq" ON "sentry_debugidartifactbundle" ("debug_id", "artifact_bundle_id", "source_file_type");
ALTER TABLE "sentry_debugidartifactbundle" ADD CONSTRAINT "sentry_debugidartifactbu_debug_id_artifact_bundle_0e34c4ff_uniq" UNIQUE USING INDEX "sentry_debugidartifactbu_debug_id_artifact_bundle_0e34c4ff_uniq";
CREATE INDEX CONCURRENTLY "sentry_debugidartifactbundle_organization_id_4db980bf" ON "sentry_debugidartifactbundle" ("organization_id");
CREATE INDEX CONCURRENTLY "sentry_debugidartifactbundle_artifact_bundle_id_91f3140f" ON "sentry_debugidartifactbundle" ("artifact_bundle_id");

@markstory
Copy link
Member

Please add the tables to the database router before merging so they are created on the files cluster since we have FK to the sentry.File table.

Related to this - the files DB cluster does not automatically run migrations and you'll need to have those migrations deployed manually, or temporarily update the files DB configuration to run migrations.

@iambriccardo
Copy link
Member Author

@markstory I had already a PR open for that, it is in draft because we first wanted to clarify the required tables.

It is available here: https://github.com/getsentry/getsentry/pull/9593

@beezz and I will sync on how we want to roll this out properly.

@@ -7,6 +7,7 @@
from .apiscopes import * # NOQA
from .apitoken import * # NOQA
from .appconnectbuilds import * # NOQA
from .artifactbundle import * # NOQA
Copy link
Member

Choose a reason for hiding this comment

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

@asottile-sentry do we still need this? I really want to at least avoid new models to end up in here.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think so -- I think this part of the patch can be discarded

__include_in_export__ = False

organization_id = BoundedBigIntegerField(db_index=True)
release_name = models.CharField(max_length=250)
Copy link
Member

Choose a reason for hiding this comment

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

Should this be release_id and related to the Release table?

Copy link
Member Author

Choose a reason for hiding this comment

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

@mitsuhiko wanted a weak reference to releases and decided to use the release.version.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 27, 2023

This PR has a migration; here is the generated SQL for src/sentry/migrations/0363_debug_id_artifact_bundle.py ()

--
-- Create model ArtifactBundle
--
CREATE TABLE "sentry_artifactbundle" ("id" bigserial NOT NULL PRIMARY KEY, "organization_id" bigint NOT NULL, "bundle_id" uuid NULL, "artifact_count" integer NOT NULL CHECK ("artifact_count" >= 0), "date_added" timestamp with time zone NOT NULL, "file_id" bigint NOT NULL);
ALTER TABLE "sentry_artifactbundle" ADD CONSTRAINT "sentry_artifactbundle_file_id_1d6752c9_fk_sentry_file_id" FOREIGN KEY ("file_id") REFERENCES "sentry_file" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "sentry_artifactbundle" VALIDATE CONSTRAINT "sentry_artifactbundle_file_id_1d6752c9_fk_sentry_file_id";
CREATE UNIQUE INDEX CONCURRENTLY "sentry_artifactbundle_organization_id_bundle_id_19f21a4b_uniq" ON "sentry_artifactbundle" ("organization_id", "bundle_id");
ALTER TABLE "sentry_artifactbundle" ADD CONSTRAINT "sentry_artifactbundle_organization_id_bundle_id_19f21a4b_uniq" UNIQUE USING INDEX "sentry_artifactbundle_organization_id_bundle_id_19f21a4b_uniq";
CREATE INDEX CONCURRENTLY "sentry_artifactbundle_organization_id_b5a13770" ON "sentry_artifactbundle" ("organization_id");
CREATE INDEX CONCURRENTLY "sentry_artifactbundle_file_id_1d6752c9" ON "sentry_artifactbundle" ("file_id");
--
-- Create model ReleaseArtifactBundle
--
CREATE TABLE "sentry_releaseartifactbundle" ("id" bigserial NOT NULL PRIMARY KEY, "organization_id" bigint NOT NULL, "release_name" varchar(250) NOT NULL, "dist_id" bigint NULL, "date_added" timestamp with time zone NOT NULL, "artifact_bundle_id" bigint NOT NULL);
ALTER TABLE "sentry_releaseartifactbundle" ADD CONSTRAINT "sentry_releaseartifa_artifact_bundle_id_e0298a12_fk_sentry_ar" FOREIGN KEY ("artifact_bundle_id") REFERENCES "sentry_artifactbundle" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "sentry_releaseartifactbundle" VALIDATE CONSTRAINT "sentry_releaseartifa_artifact_bundle_id_e0298a12_fk_sentry_ar";
CREATE UNIQUE INDEX CONCURRENTLY "sentry_releaseartifactbu_organization_id_release__ebde7736_uniq" ON "sentry_releaseartifactbundle" ("organization_id", "release_name", "dist_id", "artifact_bundle_id");
ALTER TABLE "sentry_releaseartifactbundle" ADD CONSTRAINT "sentry_releaseartifactbu_organization_id_release__ebde7736_uniq" UNIQUE USING INDEX "sentry_releaseartifactbu_organization_id_release__ebde7736_uniq";
CREATE INDEX CONCURRENTLY "sentry_releaseartifactbundle_organization_id_10fc7929" ON "sentry_releaseartifactbundle" ("organization_id");
CREATE INDEX CONCURRENTLY "sentry_releaseartifactbundle_artifact_bundle_id_e0298a12" ON "sentry_releaseartifactbundle" ("artifact_bundle_id");
--
-- Create model ProjectArtifactBundle
--
CREATE TABLE "sentry_projectartifactbundle" ("id" bigserial NOT NULL PRIMARY KEY, "organization_id" bigint NOT NULL, "project_id" bigint NOT NULL, "date_added" timestamp with time zone NOT NULL, "artifact_bundle_id" bigint NOT NULL);
ALTER TABLE "sentry_projectartifactbundle" ADD CONSTRAINT "sentry_projectartifa_artifact_bundle_id_c3f68015_fk_sentry_ar" FOREIGN KEY ("artifact_bundle_id") REFERENCES "sentry_artifactbundle" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "sentry_projectartifactbundle" VALIDATE CONSTRAINT "sentry_projectartifa_artifact_bundle_id_c3f68015_fk_sentry_ar";
CREATE UNIQUE INDEX CONCURRENTLY "sentry_projectartifactbu_project_id_artifact_bund_b374329e_uniq" ON "sentry_projectartifactbundle" ("project_id", "artifact_bundle_id");
ALTER TABLE "sentry_projectartifactbundle" ADD CONSTRAINT "sentry_projectartifactbu_project_id_artifact_bund_b374329e_uniq" UNIQUE USING INDEX "sentry_projectartifactbu_project_id_artifact_bund_b374329e_uniq";
CREATE INDEX CONCURRENTLY "sentry_projectartifactbundle_organization_id_a4b9d5f9" ON "sentry_projectartifactbundle" ("organization_id");
CREATE INDEX CONCURRENTLY "sentry_projectartifactbundle_project_id_7588084b" ON "sentry_projectartifactbundle" ("project_id");
CREATE INDEX CONCURRENTLY "sentry_projectartifactbundle_artifact_bundle_id_c3f68015" ON "sentry_projectartifactbundle" ("artifact_bundle_id");
--
-- Create model DebugIdArtifactBundle
--
CREATE TABLE "sentry_debugidartifactbundle" ("id" bigserial NOT NULL PRIMARY KEY, "organization_id" bigint NOT NULL, "debug_id" uuid NOT NULL, "source_file_type" integer NOT NULL, "date_added" timestamp with time zone NOT NULL, "date_last_accessed" timestamp with time zone NOT NULL, "artifact_bundle_id" bigint NOT NULL);
ALTER TABLE "sentry_debugidartifactbundle" ADD CONSTRAINT "sentry_debugidartifa_artifact_bundle_id_91f3140f_fk_sentry_ar" FOREIGN KEY ("artifact_bundle_id") REFERENCES "sentry_artifactbundle" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "sentry_debugidartifactbundle" VALIDATE CONSTRAINT "sentry_debugidartifa_artifact_bundle_id_91f3140f_fk_sentry_ar";
CREATE UNIQUE INDEX CONCURRENTLY "sentry_debugidartifactbu_debug_id_artifact_bundle_0e34c4ff_uniq" ON "sentry_debugidartifactbundle" ("debug_id", "artifact_bundle_id", "source_file_type");
ALTER TABLE "sentry_debugidartifactbundle" ADD CONSTRAINT "sentry_debugidartifactbu_debug_id_artifact_bundle_0e34c4ff_uniq" UNIQUE USING INDEX "sentry_debugidartifactbu_debug_id_artifact_bundle_0e34c4ff_uniq";
CREATE INDEX CONCURRENTLY "sentry_debugidartifactbundle_organization_id_4db980bf" ON "sentry_debugidartifactbundle" ("organization_id");
CREATE INDEX CONCURRENTLY "sentry_debugidartifactbundle_artifact_bundle_id_91f3140f" ON "sentry_debugidartifactbundle" ("artifact_bundle_id");

@iambriccardo iambriccardo merged commit a255e71 into master Feb 28, 2023
@iambriccardo iambriccardo deleted the riccardo/feat/debug-ids-backend branch February 28, 2023 07:25
jan-auer added a commit that referenced this pull request Feb 28, 2023
* master: (79 commits)
  feat(perf-issues): Add performance issue detection timing runner command (#44912)
  Revert "chore: Investigating org slug already set to a different value (#45134)"
  fix(hybrid-cloud): Redirect to org restoration page for customer domains (#45159)
  bug(replays): Fix 500 error when marshaling tags field (#45097)
  ref(sourcemaps): Redesign lookup of source and sourcemaps (#45032)
  chore: Investigating org slug already set to a different value (#45134)
  feat(dynamic-sampling): Implement prioritize by project bias [TET-574] (#42939)
  feat(dynamic-sampling): Add transaction name prioritize option - (#45034)
  feat(dyn-sampling): add new bias toggle to project details for prioritise by tx name [TET-717] (#44944)
  feat(admin) Add admin relay project config view [TET-509] (#45120)
  Revert "chore(assignment): Add analytics when autoassigning after a manual assignment (#45099)"
  feat(sourcemaps): Implement new tables supporting debug ids (#44572)
  ref(js): Remove usage of react-document-title (#45170)
  chore(py): Consistently name urls using `organization-` prefix (#45180)
  ref: rename acceptance required checks collector (#45156)
  chore(assignment): Add analytics when autoassigning after a manual assignment (#45099)
  feat(source-maps): Update copy for source map debug alerts (#45164)
  ref(js): Remove custom usage of DocumentTitle (#45165)
  chore(login): update the login banners (#45151)
  ref(py): Remove one more legacy project_id from Environment (#45160)
  ...
@github-actions github-actions bot locked and limited conversation to collaborators Mar 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants