Skip to content

Conversation

xvello
Copy link
Contributor

@xvello xvello commented Feb 27, 2023

Problem

Part of #14329

Changes

Support setting LIGHTWEIGHT_CAPTURE_ENDPOINT_ENABLED_TOKENS=* to enable lightweight capture on all tokens without further code changes.

We'll remove the PG lookup in a follow-up PR.

How did you test this code?

@xvello xvello requested a review from a team February 27, 2023 17:33
@@ -15,6 +15,7 @@
EVENT_PARTITION_KEYS_TO_OVERRIDE = get_list(os.getenv("EVENT_PARTITION_KEYS_TO_OVERRIDE", ""))

LIGHTWEIGHT_CAPTURE_ENDPOINT_ENABLED_TOKENS = get_list(os.getenv("LIGHTWEIGHT_CAPTURE_ENDPOINT_ENABLED_TOKENS", ""))
LIGHTWEIGHT_CAPTURE_ENDPOINT_ALL = "*" in LIGHTWEIGHT_CAPTURE_ENDPOINT_ENABLED_TOKENS
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's do this to be safe - some tokens could include * couldn't they?

Suggested change
LIGHTWEIGHT_CAPTURE_ENDPOINT_ALL = "*" in LIGHTWEIGHT_CAPTURE_ENDPOINT_ENABLED_TOKENS
LIGHTWEIGHT_CAPTURE_ENDPOINT_ALL = "*" == LIGHTWEIGHT_CAPTURE_ENDPOINT_ENABLED_TOKENS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LIGHTWEIGHT_CAPTURE_ENDPOINT_ENABLED_TOKENS (declared on the line above that) is a list, already split on the comas. We're checking that one of the items in that list is "*". This way we're less finicky than a strict envvar === "*" check.

@@ -296,7 +296,7 @@ def get_event(request):
ingestion_context = None
send_events_to_dead_letter_queue = False

if token in settings.LIGHTWEIGHT_CAPTURE_ENDPOINT_ENABLED_TOKENS:
if settings.LIGHTWEIGHT_CAPTURE_ENDPOINT_ALL or token in settings.LIGHTWEIGHT_CAPTURE_ENDPOINT_ENABLED_TOKENS:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd even just do it here since it's only used in a single place anyway

Suggested change
if settings.LIGHTWEIGHT_CAPTURE_ENDPOINT_ALL or token in settings.LIGHTWEIGHT_CAPTURE_ENDPOINT_ENABLED_TOKENS:
if settings.LIGHTWEIGHT_CAPTURE_ENDPOINT_ALL == '*' or token in settings.LIGHTWEIGHT_CAPTURE_ENDPOINT_ENABLED_TOKENS:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is executed for each event, that's why I deemed it better to check the condition once at startup.

@xvello xvello enabled auto-merge (squash) February 28, 2023 13:57
@xvello xvello merged commit a9b359b into master Feb 28, 2023
@xvello xvello deleted the xvello/lightweigth-star branch February 28, 2023 14:15
fuziontech added a commit that referenced this pull request Feb 28, 2023
* master: (53 commits)
  fix: cut the undeterministic snapshots (#14461)
  feat(hogql): Events table based on hogql (#14315)
  fix(breakdown): ensure breakdown sort can sort through values of different types (#14459)
  feat(capture): gracefully catch non-string tokens (#14453)
  chore(plugin-server): add healthcheck logging for failure (#14455)
  fix(tests): Safer migrations (#14452)
  revert: "fix(person-overrides): add constraints to catch race conditions" (#14445)
  dev(codespaces): update to python3.10 (#14449)
  chore(deps): Update posthog-js to 1.50.0 (#14448)
  feat(capture): support LIGHTWEIGHT_CAPTURE_ENDPOINT_ENABLED_TOKENS=* (#14421)
  chore: upgrade d3 (#14442)
  fix(person-overrides): add constraints to catch race conditions (#14277)
  feat(cohorts): Remove postgres calculations for flags (#14272)
  feat(capture): check token shape before team resolution too (#14439)
  feat: dashboard templates (#14322)
  feat: add a 'What's New?' button to the dropdown (#14379)
  chore(recordings): don't DLQ on PostgreSQL errors (#14438)
  chore: update autocapture attribute capture (#14435)
  chore(recordings): remove hub dependency on recordings ingestion (#14418)
  chore(deps): Update posthog-js to 1.49.0 (#14436)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants