From 70bfbd8a1433bddf2ca31e48e81613c268e6a19f Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Tue, 7 Jan 2025 14:02:11 -0800 Subject: [PATCH 1/7] ref(flags): allow launchdarkly member to be null --- src/sentry/flags/endpoints/logs.py | 4 +- .../migrations/0003_allow_null_created_by.py | 38 +++++ src/sentry/flags/models.py | 4 +- src/sentry/flags/providers.py | 15 +- tests/sentry/flags/endpoints/test_logs.py | 25 ++++ .../flags/providers/test_launchdarkly.py | 134 ++++++++++++++++++ 6 files changed, 211 insertions(+), 9 deletions(-) create mode 100644 src/sentry/flags/migrations/0003_allow_null_created_by.py diff --git a/src/sentry/flags/endpoints/logs.py b/src/sentry/flags/endpoints/logs.py index 196227873dbdd0..033d1299407ef1 100644 --- a/src/sentry/flags/endpoints/logs.py +++ b/src/sentry/flags/endpoints/logs.py @@ -21,8 +21,8 @@ class FlagAuditLogModelSerializerResponse(TypedDict): id: int action: str createdAt: datetime - createdBy: str - createdByType: str + createdBy: str | None + createdByType: str | None flag: str tags: dict[str, Any] diff --git a/src/sentry/flags/migrations/0003_allow_null_created_by.py b/src/sentry/flags/migrations/0003_allow_null_created_by.py new file mode 100644 index 00000000000000..481eda426ebaf4 --- /dev/null +++ b/src/sentry/flags/migrations/0003_allow_null_created_by.py @@ -0,0 +1,38 @@ +# Generated by Django 5.1.4 on 2025-01-07 21:56 + +from django.db import migrations, models + +from sentry.new_migrations.migrations import CheckedMigration + + +class Migration(CheckedMigration): + # This flag is used to mark that a migration shouldn't be automatically run in production. + # This should only be used for operations where it's safe to run the migration after your + # code has deployed. So this should not be used for most operations that alter the schema + # of a table. + # Here are some things that make sense to mark as post deployment: + # - Large data migrations. Typically we want these to be run manually so that they can be + # monitored and not block the deploy for a long period of time while they run. + # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to + # run this outside deployments so that we don't block them. Note that while adding an index + # is a schema change, it's completely safe to run the operation after the code has deployed. + # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment + + is_post_deployment = False + + dependencies = [ + ("flags", "0002_add_flags_webhooksigningsecret"), + ] + + operations = [ + migrations.AlterField( + model_name="flagauditlogmodel", + name="created_by", + field=models.CharField(max_length=100, null=True), + ), + migrations.AlterField( + model_name="flagauditlogmodel", + name="created_by_type", + field=models.PositiveSmallIntegerField(null=True), + ), + ] diff --git a/src/sentry/flags/models.py b/src/sentry/flags/models.py index 5e12788c61696f..141e0d4e558e13 100644 --- a/src/sentry/flags/models.py +++ b/src/sentry/flags/models.py @@ -72,8 +72,8 @@ class FlagAuditLogModel(Model): action = models.PositiveSmallIntegerField(choices=ACTION_TYPES) created_at = models.DateTimeField(default=timezone.now) - created_by = models.CharField(max_length=100) - created_by_type = models.PositiveSmallIntegerField(choices=CREATED_BY_TYPE_TYPES) + created_by = models.CharField(max_length=100, null=True) + created_by_type = models.PositiveSmallIntegerField(choices=CREATED_BY_TYPE_TYPES, null=True) flag = models.CharField(max_length=100) organization_id = HybridCloudForeignKey("sentry.Organization", null=False, on_delete="CASCADE") tags = models.JSONField() diff --git a/src/sentry/flags/providers.py b/src/sentry/flags/providers.py index 6b7e8077df4c36..312aadef3b619e 100644 --- a/src/sentry/flags/providers.py +++ b/src/sentry/flags/providers.py @@ -42,8 +42,8 @@ class FlagAuditLogRow(TypedDict): action: int created_at: datetime.datetime - created_by: str - created_by_type: int + created_by: str | None + created_by_type: int | None flag: str organization_id: int tags: dict[str, Any] @@ -92,7 +92,7 @@ def get_provider( class LaunchDarklyItemSerializer(serializers.Serializer): accesses = serializers.ListField(required=True) date = serializers.IntegerField(required=True) - member = serializers.DictField(required=True) + member = serializers.DictField(required=False, allow_null=True) name = serializers.CharField(max_length=100, required=True) description = serializers.CharField(allow_blank=True, required=True) @@ -136,14 +136,19 @@ def handle(self, message: dict[str, Any]) -> list[FlagAuditLogRow]: if access["action"] not in SUPPORTED_LAUNCHDARKLY_ACTIONS: return [] + if result.get("member"): + created_by = result["member"]["email"] + else: + created_by = None + return [ { "action": _handle_launchdarkly_actions(access["action"]), "created_at": datetime.datetime.fromtimestamp( result["date"] / 1000.0, datetime.UTC ), - "created_by": result["member"]["email"], - "created_by_type": CREATED_BY_TYPE_MAP["email"], + "created_by": created_by, + "created_by_type": CREATED_BY_TYPE_MAP["email"] if created_by else None, "flag": result["name"], "organization_id": self.organization_id, "tags": {"description": result["description"]}, diff --git a/tests/sentry/flags/endpoints/test_logs.py b/tests/sentry/flags/endpoints/test_logs.py index beaf3d03787963..f368956b428c82 100644 --- a/tests/sentry/flags/endpoints/test_logs.py +++ b/tests/sentry/flags/endpoints/test_logs.py @@ -43,6 +43,31 @@ def test_get(self): assert result["data"][0]["flag"] == "hello" assert result["data"][0]["tags"] == {"commit_sha": "123"} + def test_get_no_created_by(self): + model = FlagAuditLogModel( + action=0, + created_at=datetime.now(timezone.utc), + created_by=None, + created_by_type=None, + flag="hello", + organization_id=self.organization.id, + tags={"commit_sha": "123"}, + ) + model.save() + + with self.feature(self.features): + response = self.client.get(self.url) + assert response.status_code == 200 + + result = response.json() + assert len(result["data"]) == 1 + assert result["data"][0]["action"] == "created" + assert "createdAt" in result["data"][0] + assert result["data"][0]["createdBy"] is None + assert result["data"][0]["createdByType"] is None + assert result["data"][0]["flag"] == "hello" + assert result["data"][0]["tags"] == {"commit_sha": "123"} + def test_get_filter_by_flag(self): FlagAuditLogModel( action=0, diff --git a/tests/sentry/flags/providers/test_launchdarkly.py b/tests/sentry/flags/providers/test_launchdarkly.py index a83f4916c7a9e6..e83d423f721f02 100644 --- a/tests/sentry/flags/providers/test_launchdarkly.py +++ b/tests/sentry/flags/providers/test_launchdarkly.py @@ -351,6 +351,140 @@ def test_launchdarkly_update(): assert flag_row["action"] == ACTION_MAP["updated"] +def test_launchdarkly_create_no_member(): + request_data = { + "_links": { + "canonical": { + "href": "/api/v2/flags/default/test-flag", + "type": "application/json", + }, + "parent": {"href": "/api/v2/auditlog", "type": "application/json"}, + "self": { + "href": "/api/v2/auditlog/1234", + "type": "application/json", + }, + "site": {"href": "/default/~/features/test-flag", "type": "text/html"}, + }, + "_id": "1234", + "_accountId": "1234", + "date": 1729123465221, + "accesses": [ + {"action": "createFlag", "resource": "proj/default:env/test:flag/test-flag"}, + ], + "kind": "flag", + "name": "test flag", + "description": "flag was created", + "shortDescription": "", + "titleVerb": "created the flag", + "title": "Michelle created the flag [test flag](https://app.launchdarkly.com/default/~/features/test-flag)", + "target": { + "_links": { + "canonical": { + "href": "/api/v2/flags/default/test-flag", + "type": "application/json", + }, + "site": {"href": "/default/~/features/test-flag", "type": "text/html"}, + }, + "name": "test flag", + "resources": [ + "proj/default:env/test:flag/test-flag", + "proj/default:env/production:flag/test-flag", + ], + }, + "currentVersion": { + "name": "test flag", + "kind": "boolean", + "description": "testing a feature flag", + "key": "test-flag", + "_version": 1, + "creationDate": 1729123465176, + "includeInSnippet": False, + "clientSideAvailability": {"usingMobileKey": False, "usingEnvironmentId": False}, + "variations": [ + {"_id": "d883033e-fa8b-41d4-a4be-112d9a59278e", "value": True, "name": "on"}, + {"_id": "73aaa33f-c9ca-4bdc-8c97-01a20567aa3f", "value": False, "name": "off"}, + ], + "temporary": False, + "tags": [], + "_links": { + "parent": {"href": "/api/v2/flags/default", "type": "application/json"}, + "self": {"href": "/api/v2/flags/default/test-flag", "type": "application/json"}, + }, + "maintainerId": "1234", + "_maintainer": { + "_links": { + "self": { + "href": "/api/v2/members/1234", + "type": "application/json", + } + }, + "_id": "1234", + "firstName": "Michelle", + "lastName": "Doe", + "role": "owner", + "email": "michelle@example.com", + }, + "goalIds": [], + "experiments": {"baselineIdx": 0, "items": []}, + "customProperties": {}, + "archived": False, + "deprecated": False, + "defaults": {"onVariation": 0, "offVariation": 1}, + "environments": { + "production": { + "on": False, + "archived": False, + "salt": "1234", + "sel": "1234", + "lastModified": 1729123465190, + "version": 1, + "targets": [], + "contextTargets": [], + "rules": [], + "fallthrough": {"variation": 0}, + "offVariation": 1, + "prerequisites": [], + "_site": { + "href": "/default/production/features/test-flag", + "type": "text/html", + }, + "_environmentName": "Production", + "trackEvents": False, + "trackEventsFallthrough": False, + }, + "test": { + "on": False, + "archived": False, + "salt": "7495d3dcf72f43aaa075012fad947d0d", + "sel": "61b4861e6ed54135bc244bb120e9e2da", + "lastModified": 1729123465190, + "version": 1, + "targets": [], + "contextTargets": [], + "rules": [], + "fallthrough": {"variation": 0}, + "offVariation": 1, + "prerequisites": [], + "_site": {"href": "/default/test/features/test-flag", "type": "text/html"}, + "_environmentName": "Test", + "trackEvents": False, + "trackEventsFallthrough": False, + }, + }, + }, + } + res = LaunchDarklyProvider(123, None).handle(request_data) + assert len(res) == 1 + flag_row = res[0] + assert flag_row["action"] == ACTION_MAP["created"] + assert flag_row["flag"] == "test flag" + assert flag_row["created_by"] is None + assert flag_row["created_by_type"] is None + assert flag_row["organization_id"] == 123 + assert flag_row["tags"] is not None + assert flag_row["tags"]["description"] == "flag was created" + + def test_launchdarkly_delete_and_update(): request_data = { "_id": "1234", From ab0ad411aa7cfc3cfb031d77cdb26e4c8a211494 Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Tue, 7 Jan 2025 14:42:52 -0800 Subject: [PATCH 2/7] :white_check_mark: fix test --- src/sentry/flags/endpoints/logs.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/sentry/flags/endpoints/logs.py b/src/sentry/flags/endpoints/logs.py index 033d1299407ef1..3e2f042b055c60 100644 --- a/src/sentry/flags/endpoints/logs.py +++ b/src/sentry/flags/endpoints/logs.py @@ -35,7 +35,11 @@ def serialize(self, obj, attrs, user, **kwargs) -> FlagAuditLogModelSerializerRe "action": ActionEnum.to_string(obj.action), "createdAt": obj.created_at.isoformat(), "createdBy": obj.created_by, - "createdByType": CreatedByTypeEnum.to_string(obj.created_by_type), + "createdByType": ( + None + if obj.created_by_type is None + else CreatedByTypeEnum.to_string(obj.created_by_type) + ), "flag": obj.flag, "tags": obj.tags, } From cbb477546269627a4b09905f23945fe7eccecebf Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Wed, 8 Jan 2025 09:54:21 -0800 Subject: [PATCH 3/7] :recycle: update blueprint --- src/sentry/flags/docs/api.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/sentry/flags/docs/api.md b/src/sentry/flags/docs/api.md index 11a8313b8328ca..2e5e053d3f3924 100644 --- a/src/sentry/flags/docs/api.md +++ b/src/sentry/flags/docs/api.md @@ -29,15 +29,15 @@ Retrieve a collection of flag logs. **Attributes** -| Column | Type | Description | -| ------------- | ------ | ------------------------------------------------------------- | -| action | string | Enum of `created`, `updated`, or `deleted`. | -| createdAt | string | ISO-8601 timestamp of when the flag was changed. | -| createdBy | string | The user responsible for the change. | -| createdByType | string | Enum of `email`, `id`, or `name`. | -| flag | string | The name of the flag changed. Maps to flag_log_id in the URI. | -| id | number | A unique identifier for the log entry. | -| tags | object | A collection of provider-specified scoping metadata. | +| Column | Type | Description | +| ------------- | -------------- | ------------------------------------------------------------- | +| action | string | Enum of `created`, `updated`, or `deleted`. | +| createdAt | string | ISO-8601 timestamp of when the flag was changed. | +| createdBy | string or null | The user responsible for the change. | +| createdByType | string or null | Enum of `email`, `id`, or `name`. | +| flag | string | The name of the flag changed. Maps to flag_log_id in the URI. | +| id | number | A unique identifier for the log entry. | +| tags | object | A collection of provider-specified scoping metadata. | - Response 200 From 73492201c379169fc51d2a0b4f585ce56b0f78bf Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Wed, 8 Jan 2025 12:28:39 -0800 Subject: [PATCH 4/7] Update src/sentry/flags/docs/api.md --- src/sentry/flags/docs/api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/flags/docs/api.md b/src/sentry/flags/docs/api.md index 2e5e053d3f3924..19991a43dffe19 100644 --- a/src/sentry/flags/docs/api.md +++ b/src/sentry/flags/docs/api.md @@ -34,7 +34,7 @@ Retrieve a collection of flag logs. | action | string | Enum of `created`, `updated`, or `deleted`. | | createdAt | string | ISO-8601 timestamp of when the flag was changed. | | createdBy | string or null | The user responsible for the change. | -| createdByType | string or null | Enum of `email`, `id`, or `name`. | +| createdByType | optional[string] | Enum of `email`, `id`, or `name`. | | flag | string | The name of the flag changed. Maps to flag_log_id in the URI. | | id | number | A unique identifier for the log entry. | | tags | object | A collection of provider-specified scoping metadata. | From 529679f0ae2a5e6c7a56e4dfb06487bfcecdb63f Mon Sep 17 00:00:00 2001 From: "getsantry[bot]" <66042841+getsantry[bot]@users.noreply.github.com> Date: Wed, 8 Jan 2025 20:29:35 +0000 Subject: [PATCH 5/7] :hammer_and_wrench: apply pre-commit fixes --- src/sentry/flags/docs/api.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/sentry/flags/docs/api.md b/src/sentry/flags/docs/api.md index 19991a43dffe19..e4e0e9f56ccc68 100644 --- a/src/sentry/flags/docs/api.md +++ b/src/sentry/flags/docs/api.md @@ -29,15 +29,15 @@ Retrieve a collection of flag logs. **Attributes** -| Column | Type | Description | -| ------------- | -------------- | ------------------------------------------------------------- | -| action | string | Enum of `created`, `updated`, or `deleted`. | -| createdAt | string | ISO-8601 timestamp of when the flag was changed. | -| createdBy | string or null | The user responsible for the change. | +| Column | Type | Description | +| ------------- | ---------------- | ------------------------------------------------------------- | +| action | string | Enum of `created`, `updated`, or `deleted`. | +| createdAt | string | ISO-8601 timestamp of when the flag was changed. | +| createdBy | string or null | The user responsible for the change. | | createdByType | optional[string] | Enum of `email`, `id`, or `name`. | -| flag | string | The name of the flag changed. Maps to flag_log_id in the URI. | -| id | number | A unique identifier for the log entry. | -| tags | object | A collection of provider-specified scoping metadata. | +| flag | string | The name of the flag changed. Maps to flag_log_id in the URI. | +| id | number | A unique identifier for the log entry. | +| tags | object | A collection of provider-specified scoping metadata. | - Response 200 From c945f68c9dded0638a95f1acd55f7a782c00ccad Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Wed, 8 Jan 2025 12:29:59 -0800 Subject: [PATCH 6/7] Update src/sentry/flags/docs/api.md --- src/sentry/flags/docs/api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/flags/docs/api.md b/src/sentry/flags/docs/api.md index e4e0e9f56ccc68..6e617f347d35a3 100644 --- a/src/sentry/flags/docs/api.md +++ b/src/sentry/flags/docs/api.md @@ -33,7 +33,7 @@ Retrieve a collection of flag logs. | ------------- | ---------------- | ------------------------------------------------------------- | | action | string | Enum of `created`, `updated`, or `deleted`. | | createdAt | string | ISO-8601 timestamp of when the flag was changed. | -| createdBy | string or null | The user responsible for the change. | +| createdBy | optional[string] | The user responsible for the change. | | createdByType | optional[string] | Enum of `email`, `id`, or `name`. | | flag | string | The name of the flag changed. Maps to flag_log_id in the URI. | | id | number | A unique identifier for the log entry. | From 0457192c5f4d8a9e8c6cf8617ba04d31d4b85a53 Mon Sep 17 00:00:00 2001 From: "getsantry[bot]" <66042841+getsantry[bot]@users.noreply.github.com> Date: Wed, 8 Jan 2025 20:31:02 +0000 Subject: [PATCH 7/7] :hammer_and_wrench: apply pre-commit fixes --- src/sentry/flags/docs/api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/flags/docs/api.md b/src/sentry/flags/docs/api.md index 6e617f347d35a3..e16a5731ff1b23 100644 --- a/src/sentry/flags/docs/api.md +++ b/src/sentry/flags/docs/api.md @@ -33,7 +33,7 @@ Retrieve a collection of flag logs. | ------------- | ---------------- | ------------------------------------------------------------- | | action | string | Enum of `created`, `updated`, or `deleted`. | | createdAt | string | ISO-8601 timestamp of when the flag was changed. | -| createdBy | optional[string] | The user responsible for the change. | +| createdBy | optional[string] | The user responsible for the change. | | createdByType | optional[string] | Enum of `email`, `id`, or `name`. | | flag | string | The name of the flag changed. Maps to flag_log_id in the URI. | | id | number | A unique identifier for the log entry. |