diff --git a/bin/deploy-hobby b/bin/deploy-hobby index 1f790dc7ba147..93df1b1aac3a6 100755 --- a/bin/deploy-hobby +++ b/bin/deploy-hobby @@ -12,6 +12,9 @@ export SENTRY_DSN="${SENTRY_DSN:-'https://public@sentry.example.com/1'}" POSTHOG_SECRET=$(head -c 28 /dev/urandom | sha224sum -b | head -c 56) export POSTHOG_SECRET +ENCRYPTION_KEY=$(openssl rand -hex 16) +export ENCRYPTION_SALT_KEYS + # Talk to the user echo "Welcome to the single instance PostHog installer 🦔" echo "" @@ -128,6 +131,7 @@ EOF # Write .env file envsubst > .env </dev/null 2>&1; then + kill "$worker_pid" + else + echo "Worker process is not running." + fi +} -python3 manage.py start_temporal_worker "$@" +trap cleanup SIGINT SIGTERM EXIT -wait +python3 manage.py start_temporal_worker "$@" & + +worker_pid=$! + +wait $worker_pid + +cleanup diff --git a/bin/upgrade-hobby b/bin/upgrade-hobby old mode 100644 new mode 100755 index 2c882978618e9..71e74ddbb5c96 --- a/bin/upgrade-hobby +++ b/bin/upgrade-hobby @@ -3,7 +3,7 @@ set -e echo "Upgrading PostHog. This will cause a few minutes of downtime." -read -r -p "Do you want to upgarde PostHog? [y/N] " response +read -r -p "Do you want to upgrade PostHog? [y/N] " response if [[ "$response" =~ ^([yY][eE][sS]|[yY])+$ ]] then echo "OK!" @@ -56,6 +56,28 @@ else fi [[ -f ".env" ]] && export $(cat .env | xargs) || ( echo "No .env file found. Please create it with POSTHOG_SECRET and DOMAIN set." && exit 1) + +# we introduced ENCRYPTION_SALT_KEYS and so if there isn't one, need to add it +# check for it in the .env file +if ! grep -q "ENCRYPTION_SALT_KEYS" .env; then + ENCRYPTION_KEY=$(openssl rand -hex 16) + echo "ENCRYPTION_SALT_KEYS=$ENCRYPTION_KEY" >> .env + echo "Added missing ENCRYPTION_SALT_KEYS to .env file" + source .env +else + # Read the existing key + EXISTING_KEY=$(grep "ENCRYPTION_SALT_KEYS" .env | cut -d '=' -f2) + + # Check if the existing key is in the correct format (32 bytes base64url) + if [[ ! $EXISTING_KEY =~ ^[A-Za-z0-9_-]{32}$ ]]; then + echo "ENCRYPTION_SALT_KEYS is not in the correct fernet format and will not work" + echo "🛑 Stop this script and do not proceed" + echo "remove ENCRYPTION_SALT_KEYS from .env and try again" + exit 1 + fi +fi + + export POSTHOG_APP_TAG="${POSTHOG_APP_TAG:-latest-release}" cd posthog diff --git a/docker-compose.hobby.yml b/docker-compose.hobby.yml index 7df84ca8035cd..144561399629f 100644 --- a/docker-compose.hobby.yml +++ b/docker-compose.hobby.yml @@ -83,6 +83,7 @@ services: OBJECT_STORAGE_SECRET_ACCESS_KEY: 'object_storage_root_password' OBJECT_STORAGE_ENDPOINT: http://objectstorage:19000 OBJECT_STORAGE_ENABLED: true + ENCRYPTION_SALT_KEYS: $ENCRYPTION_SALT_KEYS image: $REGISTRY_URL:$POSTHOG_APP_TAG web: extends: @@ -100,6 +101,7 @@ services: OBJECT_STORAGE_SECRET_ACCESS_KEY: 'object_storage_root_password' OBJECT_STORAGE_ENDPOINT: http://objectstorage:19000 OBJECT_STORAGE_ENABLED: true + ENCRYPTION_SALT_KEYS: $ENCRYPTION_SALT_KEYS depends_on: - db - redis @@ -122,6 +124,7 @@ services: OBJECT_STORAGE_ENABLED: true CDP_REDIS_HOST: redis7 CDP_REDIS_PORT: 6379 + ENCRYPTION_SALT_KEYS: $ENCRYPTION_SALT_KEYS depends_on: - db - redis diff --git a/docker/clickhouse/docker-entrypoint-initdb.d/init-db.sh b/docker/clickhouse/docker-entrypoint-initdb.d/init-db.sh index 977d436f97635..4801f08e1f9c6 100755 --- a/docker/clickhouse/docker-entrypoint-initdb.d/init-db.sh +++ b/docker/clickhouse/docker-entrypoint-initdb.d/init-db.sh @@ -3,5 +3,10 @@ set -e apt-get update apt-get -y install python3.9 -ln -s /usr/bin/python3.9 /usr/bin/python3 -cp -r /idl/* /var/lib/clickhouse/format_schemas/ + +# Check if /usr/bin/python3 already exists and if it points to python3.9 +if [[ $(readlink /usr/bin/python3) != "/usr/bin/python3.9" ]]; then + ln -sf /usr/bin/python3.9 /usr/bin/python3 +fi + +cp -r /idl/* /var/lib/clickhouse/format_schemas/ \ No newline at end of file diff --git a/ee/api/billing.py b/ee/api/billing.py index 61dfef576259a..be5076721291f 100644 --- a/ee/api/billing.py +++ b/ee/api/billing.py @@ -11,6 +11,7 @@ from rest_framework.exceptions import NotFound, PermissionDenied, ValidationError from rest_framework.request import Request from rest_framework.response import Response +from django.contrib.auth.models import AbstractUser from ee.billing.billing_manager import BillingManager, build_billing_token from ee.models import License @@ -40,6 +41,13 @@ class BillingViewset(TeamAndOrgViewSetMixin, viewsets.GenericViewSet): scope_object = "INTERNAL" + def get_billing_manager(self) -> BillingManager: + license = get_cached_instance_license() + user = ( + self.request.user if isinstance(self.request.user, AbstractUser) and self.request.user.distinct_id else None + ) + return BillingManager(license, user) + def list(self, request: Request, *args: Any, **kwargs: Any) -> Response: license = get_cached_instance_license() if license and not license.is_v2_license: @@ -53,7 +61,8 @@ def list(self, request: Request, *args: Any, **kwargs: Any) -> Response: raise NotFound("Billing V1 is active for this organization") plan_keys = request.query_params.get("plan_keys", None) - response = BillingManager(license).get_billing(org, plan_keys) + billing_manager = self.get_billing_manager() + response = billing_manager.get_billing(org, plan_keys) return Response(response) @@ -68,7 +77,8 @@ def patch(self, request: Request, *args: Any, **kwargs: Any) -> Response: if license and org: # for mypy custom_limits_usd = request.data.get("custom_limits_usd") if custom_limits_usd: - BillingManager(license).update_billing(org, {"custom_limits_usd": custom_limits_usd}) + billing_manager = self.get_billing_manager() + billing_manager.update_billing(org, {"custom_limits_usd": custom_limits_usd}) if distinct_id: posthoganalytics.capture( @@ -153,7 +163,6 @@ class DeactivateSerializer(serializers.Serializer): @action(methods=["GET"], detail=False) def deactivate(self, request: Request, *args: Any, **kwargs: Any) -> HttpResponse: - license = get_cached_instance_license() organization = self._get_org_required() serializer = self.DeactivateSerializer(data=request.GET) @@ -162,7 +171,8 @@ def deactivate(self, request: Request, *args: Any, **kwargs: Any) -> HttpRespons products = serializer.validated_data.get("products") try: - BillingManager(license).deactivate_products(organization, products) + billing_manager = self.get_billing_manager() + billing_manager.deactivate_products(organization, products) except Exception as e: if len(e.args) > 2: detail_object = e.args[2] @@ -191,7 +201,8 @@ def portal(self, request: Request, *args: Any, **kwargs: Any) -> HttpResponse: organization = self._get_org_required() - res = BillingManager(license)._get_stripe_portal_url(organization) + billing_manager = self.get_billing_manager() + res = billing_manager._get_stripe_portal_url(organization) return redirect(res) @action(methods=["GET"], detail=False) @@ -208,7 +219,8 @@ def get_invoices(self, request: Request, *args: Any, **kwargs: Any) -> HttpRespo invoice_status = request.GET.get("status") try: - res = BillingManager(license).get_invoices(organization, status=invoice_status) + billing_manager = self.get_billing_manager() + res = billing_manager.get_invoices(organization, status=invoice_status) except Exception as e: if len(e.args) > 2: detail_object = e.args[2] @@ -244,7 +256,8 @@ def credits_overview(self, request: Request, *args: Any, **kwargs: Any) -> HttpR organization = self._get_org_required() - res = BillingManager(license).credits_overview(organization) + billing_manager = self.get_billing_manager() + res = billing_manager.credits_overview(organization) return Response(res, status=status.HTTP_200_OK) @action(methods=["POST"], detail=False, url_path="credits/purchase") @@ -258,7 +271,8 @@ def purchase_credits(self, request: Request, *args: Any, **kwargs: Any) -> HttpR organization = self._get_org_required() - res = BillingManager(license).purchase_credits(organization, request.data) + billing_manager = self.get_billing_manager() + res = billing_manager.purchase_credits(organization, request.data) return Response(res, status=status.HTTP_200_OK) @action(methods=["PATCH"], detail=False) diff --git a/ee/api/test/test_billing.py b/ee/api/test/test_billing.py index a053dd1ac9c1c..2875145441d69 100644 --- a/ee/api/test/test_billing.py +++ b/ee/api/test/test_billing.py @@ -318,6 +318,7 @@ def mock_implementation(url: str, headers: Any = None, params: Any = None) -> Ma assert decoded_token == { "aud": "posthog:license-key", + "distinct_id": str(self.user.distinct_id), "exp": 1640996100, "id": self.license.key.split("::")[0], "organization_id": str(self.organization.id), diff --git a/ee/billing/billing_manager.py b/ee/billing/billing_manager.py index 9e9d4ebb6c5a6..3e179237162d1 100644 --- a/ee/billing/billing_manager.py +++ b/ee/billing/billing_manager.py @@ -20,6 +20,7 @@ from posthog.cloud_utils import get_cached_instance_license from posthog.models import Organization from posthog.models.organization import OrganizationMembership, OrganizationUsageInfo +from posthog.models.user import User logger = structlog.get_logger(__name__) @@ -28,21 +29,26 @@ class BillingAPIErrorCodes(Enum): OPEN_INVOICES_ERROR = "open_invoices_error" -def build_billing_token(license: License, organization: Organization): +def build_billing_token(license: License, organization: Organization, user: Optional[User] = None): if not organization or not license: raise NotAuthenticated() license_id = license.key.split("::")[0] license_secret = license.key.split("::")[1] + payload = { + "exp": datetime.now(tz=timezone.utc) + timedelta(minutes=15), + "id": license_id, + "organization_id": str(organization.id), + "organization_name": organization.name, + "aud": "posthog:license-key", + } + + if user: + payload["distinct_id"] = str(user.distinct_id) + encoded_jwt = jwt.encode( - { - "exp": datetime.now(tz=timezone.utc) + timedelta(minutes=15), - "id": license_id, - "organization_id": str(organization.id), - "organization_name": organization.name, - "aud": "posthog:license-key", - }, + payload, license_secret, algorithm="HS256", ) @@ -62,9 +68,11 @@ def handle_billing_service_error(res: requests.Response, valid_codes=(200, 404, class BillingManager: license: Optional[License] + user: Optional[User] - def __init__(self, license): + def __init__(self, license, user: Optional[User] = None): self.license = license or get_cached_instance_license() + self.user = user def get_billing(self, organization: Optional[Organization], plan_keys: Optional[str]) -> dict[str, Any]: if organization and self.license and self.license.is_v2_license: @@ -331,7 +339,7 @@ def update_org_details(self, organization: Organization, billing_status: Billing def get_auth_headers(self, organization: Organization): if not self.license: # mypy raise Exception("No license found") - billing_service_token = build_billing_token(self.license, organization) + billing_service_token = build_billing_token(self.license, organization, self.user) return {"Authorization": f"Bearer {billing_service_token}"} def get_invoices(self, organization: Organization, status: Optional[str]): diff --git a/frontend/__snapshots__/lemon-ui-lemon-input-select--default--dark.png b/frontend/__snapshots__/lemon-ui-lemon-input-select--default--dark.png index ec8494f1cb683..30d6848c0dcbf 100644 Binary files a/frontend/__snapshots__/lemon-ui-lemon-input-select--default--dark.png and b/frontend/__snapshots__/lemon-ui-lemon-input-select--default--dark.png differ diff --git a/frontend/__snapshots__/lemon-ui-lemon-input-select--default--light.png b/frontend/__snapshots__/lemon-ui-lemon-input-select--default--light.png index 5496b3c4488fa..115f6d83d8d7f 100644 Binary files a/frontend/__snapshots__/lemon-ui-lemon-input-select--default--light.png and b/frontend/__snapshots__/lemon-ui-lemon-input-select--default--light.png differ diff --git a/frontend/__snapshots__/scenes-app-errortracking--group-page--dark.png b/frontend/__snapshots__/scenes-app-errortracking--group-page--dark.png index 9ba7eb52e5ca6..65789fcd50fe2 100644 Binary files a/frontend/__snapshots__/scenes-app-errortracking--group-page--dark.png and b/frontend/__snapshots__/scenes-app-errortracking--group-page--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-errortracking--group-page--light.png b/frontend/__snapshots__/scenes-app-errortracking--group-page--light.png index 6e01906523ab6..0d0cd6d752a46 100644 Binary files a/frontend/__snapshots__/scenes-app-errortracking--group-page--light.png and b/frontend/__snapshots__/scenes-app-errortracking--group-page--light.png differ diff --git a/frontend/__snapshots__/scenes-app-errortracking--list-page--dark.png b/frontend/__snapshots__/scenes-app-errortracking--list-page--dark.png index 70c8fa237a9ba..476a6507dab73 100644 Binary files a/frontend/__snapshots__/scenes-app-errortracking--list-page--dark.png and b/frontend/__snapshots__/scenes-app-errortracking--list-page--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-errortracking--list-page--light.png b/frontend/__snapshots__/scenes-app-errortracking--list-page--light.png index d23dbf662cde5..c3e70a0523ba4 100644 Binary files a/frontend/__snapshots__/scenes-app-errortracking--list-page--light.png and b/frontend/__snapshots__/scenes-app-errortracking--list-page--light.png differ diff --git a/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--dark.png b/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--dark.png index 74051399b3cea..5aca2a8a4304f 100644 Binary files a/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--dark.png and b/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--light.png b/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--light.png index c25e145613bf2..08eda84564d9d 100644 Binary files a/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--light.png and b/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--light.png differ diff --git a/frontend/__snapshots__/scenes-app-feature-flags--edit-multi-variate-feature-flag--dark.png b/frontend/__snapshots__/scenes-app-feature-flags--edit-multi-variate-feature-flag--dark.png index f1ce2ae3a9eb4..8d603656da78c 100644 Binary files a/frontend/__snapshots__/scenes-app-feature-flags--edit-multi-variate-feature-flag--dark.png and b/frontend/__snapshots__/scenes-app-feature-flags--edit-multi-variate-feature-flag--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-feature-flags--edit-multi-variate-feature-flag--light.png b/frontend/__snapshots__/scenes-app-feature-flags--edit-multi-variate-feature-flag--light.png index 13b0f4a86338d..8c9c13326add9 100644 Binary files a/frontend/__snapshots__/scenes-app-feature-flags--edit-multi-variate-feature-flag--light.png and b/frontend/__snapshots__/scenes-app-feature-flags--edit-multi-variate-feature-flag--light.png differ diff --git a/frontend/__snapshots__/scenes-app-feature-flags--new-feature-flag--dark.png b/frontend/__snapshots__/scenes-app-feature-flags--new-feature-flag--dark.png index 376f144b2a269..62418d8fb584a 100644 Binary files a/frontend/__snapshots__/scenes-app-feature-flags--new-feature-flag--dark.png and b/frontend/__snapshots__/scenes-app-feature-flags--new-feature-flag--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-feature-flags--new-feature-flag--light.png b/frontend/__snapshots__/scenes-app-feature-flags--new-feature-flag--light.png index 0200cd5036e53..404b840975200 100644 Binary files a/frontend/__snapshots__/scenes-app-feature-flags--new-feature-flag--light.png and b/frontend/__snapshots__/scenes-app-feature-flags--new-feature-flag--light.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--light.png b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--light.png index 78ea79ea3f745..74e8409c16b8b 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--light.png and b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--light.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--user-paths-edit--light.png b/frontend/__snapshots__/scenes-app-insights--user-paths-edit--light.png index aa8524427af0c..87356fa9f2e2b 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--user-paths-edit--light.png and b/frontend/__snapshots__/scenes-app-insights--user-paths-edit--light.png differ diff --git a/frontend/__snapshots__/scenes-app-persons-groups--cohorts--dark.png b/frontend/__snapshots__/scenes-app-persons-groups--cohorts--dark.png index 4bf992dd16a86..2b513355fd164 100644 Binary files a/frontend/__snapshots__/scenes-app-persons-groups--cohorts--dark.png and b/frontend/__snapshots__/scenes-app-persons-groups--cohorts--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-persons-groups--cohorts--light.png b/frontend/__snapshots__/scenes-app-persons-groups--cohorts--light.png index 03987efb3b8c6..785f6e01082a6 100644 Binary files a/frontend/__snapshots__/scenes-app-persons-groups--cohorts--light.png and b/frontend/__snapshots__/scenes-app-persons-groups--cohorts--light.png differ diff --git a/frontend/__snapshots__/scenes-app-persons-groups--persons--dark.png b/frontend/__snapshots__/scenes-app-persons-groups--persons--dark.png index 97c41b19b5893..a30eac7b66913 100644 Binary files a/frontend/__snapshots__/scenes-app-persons-groups--persons--dark.png and b/frontend/__snapshots__/scenes-app-persons-groups--persons--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-persons-groups--persons--light.png b/frontend/__snapshots__/scenes-app-persons-groups--persons--light.png index ee860c71cc823..eeae899249b91 100644 Binary files a/frontend/__snapshots__/scenes-app-persons-groups--persons--light.png and b/frontend/__snapshots__/scenes-app-persons-groups--persons--light.png differ diff --git a/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-new-hog-function--light.png b/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-new-hog-function--light.png index c7b134087fa21..3110451381835 100644 Binary files a/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-new-hog-function--light.png and b/frontend/__snapshots__/scenes-app-pipeline--pipeline-node-new-hog-function--light.png differ diff --git a/frontend/src/layout/navigation-3000/sidepanel/SidePanel.tsx b/frontend/src/layout/navigation-3000/sidepanel/SidePanel.tsx index 34c18f4fc6ff2..972ca1515de48 100644 --- a/frontend/src/layout/navigation-3000/sidepanel/SidePanel.tsx +++ b/frontend/src/layout/navigation-3000/sidepanel/SidePanel.tsx @@ -1,6 +1,6 @@ import './SidePanel.scss' -import { IconEllipsis, IconFeatures, IconGear, IconInfo, IconNotebook, IconSupport } from '@posthog/icons' +import { IconEllipsis, IconFeatures, IconFlag, IconGear, IconInfo, IconNotebook, IconSupport } from '@posthog/icons' import { LemonButton, LemonMenu, LemonMenuItems, LemonModal } from '@posthog/lemon-ui' import clsx from 'clsx' import { useActions, useValues } from 'kea' @@ -20,6 +20,7 @@ import { SidePanelActivation, SidePanelActivationIcon } from './panels/activatio import { SidePanelActivity, SidePanelActivityIcon } from './panels/activity/SidePanelActivity' import { SidePanelDiscussion, SidePanelDiscussionIcon } from './panels/discussion/SidePanelDiscussion' import { SidePanelDocs } from './panels/SidePanelDocs' +import { SidePanelExperimentFeatureFlag } from './panels/SidePanelExperimentFeatureFlag' import { SidePanelFeaturePreviews } from './panels/SidePanelFeaturePreviews' import { SidePanelSettings } from './panels/SidePanelSettings' import { SidePanelStatus, SidePanelStatusIcon } from './panels/SidePanelStatus' @@ -87,6 +88,11 @@ export const SIDE_PANEL_TABS: Record< Content: SidePanelStatus, noModalSupport: true, }, + [SidePanelTab.ExperimentFeatureFlag]: { + label: 'Release conditions', + Icon: IconFlag, + Content: SidePanelExperimentFeatureFlag, + }, } const DEFAULT_WIDTH = 512 diff --git a/frontend/src/layout/navigation-3000/sidepanel/panels/SidePanelExperimentFeatureFlag.tsx b/frontend/src/layout/navigation-3000/sidepanel/panels/SidePanelExperimentFeatureFlag.tsx new file mode 100644 index 0000000000000..e15c10ce7b0e2 --- /dev/null +++ b/frontend/src/layout/navigation-3000/sidepanel/panels/SidePanelExperimentFeatureFlag.tsx @@ -0,0 +1,156 @@ +import { LemonBanner, LemonButton, LemonDivider, LemonInput, LemonTable, Link, Spinner } from '@posthog/lemon-ui' +import { useActions, useValues } from 'kea' +import { router } from 'kea-router' +import { useEffect, useMemo } from 'react' +import { experimentLogic } from 'scenes/experiments/experimentLogic' +import { featureFlagLogic, FeatureFlagLogicProps } from 'scenes/feature-flags/featureFlagLogic' +import { FeatureFlagReleaseConditions } from 'scenes/feature-flags/FeatureFlagReleaseConditions' +import { urls } from 'scenes/urls' + +import { sidePanelStateLogic } from '../sidePanelStateLogic' + +export const SidePanelExperimentFeatureFlag = (): JSX.Element => { + const { closeSidePanel } = useActions(sidePanelStateLogic) + const { currentLocation } = useValues(router) + + useEffect(() => { + // Side panel state is persisted in local storage, so we need to check if we're on the experiment page, + // otherwise close the side panel + const isExperimentPath = /^\/project\/[0-9]+\/experiments\/[0-9]+/.test(currentLocation.pathname) + if (!isExperimentPath) { + closeSidePanel() + } + }, [currentLocation, closeSidePanel]) + + // Retrieve experiment ID from URL + const experimentId = useMemo(() => { + const match = currentLocation.pathname.match(/\/experiments\/(\d+)/) + return match ? parseInt(match[1]) : null + }, [currentLocation.pathname]) + + const { experiment } = useValues(experimentLogic({ experimentId: experimentId ?? 'new' })) + + const _featureFlagLogic = featureFlagLogic({ id: experiment.feature_flag?.id ?? null } as FeatureFlagLogicProps) + const { featureFlag, areVariantRolloutsValid, variantRolloutSum, featureFlagLoading } = useValues(_featureFlagLogic) + const { setFeatureFlagFilters, saveSidebarExperimentFeatureFlag, distributeVariantsEqually } = + useActions(_featureFlagLogic) + + const variants = featureFlag?.filters?.multivariate?.variants || [] + + const handleRolloutPercentageChange = (index: number, value: number | undefined): void => { + if (!featureFlag?.filters?.multivariate || !value) { + return + } + + const updatedVariants = featureFlag.filters.multivariate.variants.map((variant, i) => + i === index ? { ...variant, rollout_percentage: value } : variant + ) + + const updatedFilters = { + ...featureFlag.filters, + multivariate: { ...featureFlag.filters.multivariate, variants: updatedVariants }, + } + + setFeatureFlagFilters(updatedFilters, null) + } + + if (featureFlagLoading || !featureFlag.id) { + return ( +
+ +
+ ) + } + + return ( +
+ +
+
+ Adjusting variant distribution or user targeting may impact the validity of your results. Adjust + only if you're aware of how changes will affect your experiment. +
+
+ For full feature flag settings, go to{' '} + + {experiment.feature_flag?.key} + {' '} + . +
+
+ +
+

Experiment variants

+ {value}, + width: '50%', + }, + { + title: ( +
+ Rollout Percentage + + Redistribute + +
+ ), + dataIndex: 'rollout_percentage', + key: 'rollout_percentage', + render: (_, record, index) => ( + { + if (changedValue !== null) { + const valueInt = + changedValue !== undefined ? parseInt(changedValue.toString()) : 0 + if (!isNaN(valueInt)) { + handleRolloutPercentageChange(index, changedValue) + } + } + }} + min={0} + max={100} + /> + ), + }, + ]} + /> + {variants.length > 0 && !areVariantRolloutsValid && ( +

+ Percentage rollouts for variants must sum to 100 (currently {variantRolloutSum} + ). +

+ )} +
+ + + +
+ { + saveSidebarExperimentFeatureFlag(featureFlag) + }} + > + Save + +
+
+ ) +} diff --git a/frontend/src/layout/navigation-3000/sidepanel/sidePanelLogic.tsx b/frontend/src/layout/navigation-3000/sidepanel/sidePanelLogic.tsx index 029b34b6cbf4a..2a4add974a1d9 100644 --- a/frontend/src/layout/navigation-3000/sidepanel/sidePanelLogic.tsx +++ b/frontend/src/layout/navigation-3000/sidepanel/sidePanelLogic.tsx @@ -1,4 +1,5 @@ import { connect, kea, path, selectors } from 'kea' +import { router } from 'kea-router' import { FEATURE_FLAGS } from 'lib/constants' import { featureFlagLogic } from 'lib/logic/featureFlagLogic' import { preflightLogic } from 'scenes/PreflightCheck/preflightLogic' @@ -39,6 +40,8 @@ export const sidePanelLogic = kea([ ['status'], userLogic, ['hasAvailableFeature'], + router, + ['currentLocation'], ], actions: [sidePanelStateLogic, ['closeSidePanel', 'openSidePanel']], }), @@ -49,6 +52,7 @@ export const sidePanelLogic = kea([ (isCloudOrDev, isReady, hasCompletedAllTasks, featureflags) => { const tabs: SidePanelTab[] = [] + tabs.push(SidePanelTab.ExperimentFeatureFlag) tabs.push(SidePanelTab.Notebooks) tabs.push(SidePanelTab.Docs) if (isCloudOrDev) { @@ -74,8 +78,24 @@ export const sidePanelLogic = kea([ ], visibleTabs: [ - (s) => [s.enabledTabs, s.selectedTab, s.sidePanelOpen, s.unreadCount, s.status, s.hasAvailableFeature], - (enabledTabs, selectedTab, sidePanelOpen, unreadCount, status, hasAvailableFeature): SidePanelTab[] => { + (s) => [ + s.enabledTabs, + s.selectedTab, + s.sidePanelOpen, + s.unreadCount, + s.status, + s.hasAvailableFeature, + s.currentLocation, + ], + ( + enabledTabs, + selectedTab, + sidePanelOpen, + unreadCount, + status, + hasAvailableFeature, + currentLocation + ): SidePanelTab[] => { return enabledTabs.filter((tab) => { if (tab === selectedTab && sidePanelOpen) { return true @@ -98,6 +118,10 @@ export const sidePanelLogic = kea([ return false } + if (tab === SidePanelTab.ExperimentFeatureFlag) { + return /^\/project\/[0-9]+\/experiments\/[0-9]+/.test(currentLocation.pathname) + } + return true }) }, diff --git a/frontend/src/lib/components/AuthorizedUrlList/authorizedUrlListLogic.test.ts b/frontend/src/lib/components/AuthorizedUrlList/authorizedUrlListLogic.test.ts index db677f638cb00..b8678715352ea 100644 --- a/frontend/src/lib/components/AuthorizedUrlList/authorizedUrlListLogic.test.ts +++ b/frontend/src/lib/components/AuthorizedUrlList/authorizedUrlListLogic.test.ts @@ -85,6 +85,14 @@ describe('the authorized urls list logic', () => { proposedUrl: 'https://not.*.valid.*', validityMessage: 'Wildcards can only be used for subdomains', }, + { + proposedUrl: 'http://localhost:*', + validityMessage: 'Wildcards are not allowed in the port position', + }, + { + proposedUrl: 'http://valid.example.com:*', + validityMessage: 'Wildcards are not allowed in the port position', + }, ] testCases.forEach((testCase) => { diff --git a/frontend/src/lib/components/AuthorizedUrlList/authorizedUrlListLogic.ts b/frontend/src/lib/components/AuthorizedUrlList/authorizedUrlListLogic.ts index 8cc977e20f2ae..29225f3d7d4ec 100644 --- a/frontend/src/lib/components/AuthorizedUrlList/authorizedUrlListLogic.ts +++ b/frontend/src/lib/components/AuthorizedUrlList/authorizedUrlListLogic.ts @@ -47,6 +47,18 @@ export function sanitizePossibleWildCardedURL(url: string): URL { return new URL(deWildCardedURL) } +/** + * Checks if the URL has a wildcard (*) in the port position eg http://localhost:* + */ +export function hasPortWildcard(input: string): boolean { + if (!input || typeof input !== 'string') { + return false + } + // This regex matches URLs with a wildcard (*) in the port position + const portWildcardRegex = /^(https?:\/\/[^:/]+):\*(.*)$/ + return portWildcardRegex.test(input.trim()) +} + export const validateProposedUrl = ( proposedUrl: string, currentUrls: string[], @@ -56,6 +68,10 @@ export const validateProposedUrl = ( return 'Please enter a valid URL' } + if (hasPortWildcard(proposedUrl)) { + return 'Wildcards are not allowed in the port position' + } + if (onlyAllowDomains && !isDomain(sanitizePossibleWildCardedURL(proposedUrl))) { return "Please enter a valid domain (URLs with a path aren't allowed)" } diff --git a/frontend/src/lib/components/Cards/InsightCard/InsightCard.tsx b/frontend/src/lib/components/Cards/InsightCard/InsightCard.tsx index 31f9243d986f8..474665182957f 100644 --- a/frontend/src/lib/components/Cards/InsightCard/InsightCard.tsx +++ b/frontend/src/lib/components/Cards/InsightCard/InsightCard.tsx @@ -11,7 +11,6 @@ import { insightLogic } from 'scenes/insights/insightLogic' import { ErrorBoundary } from '~/layout/ErrorBoundary' import { themeLogic } from '~/layout/navigation-3000/themeLogic' import { Query } from '~/queries/Query/Query' -import { DashboardFilter } from '~/queries/schema' import { DashboardBasicType, DashboardPlacement, @@ -61,8 +60,6 @@ export interface InsightCardProps extends Resizeable, React.HTMLAttributes @@ -145,7 +141,6 @@ function InsightCardInternal( showEditingControls={showEditingControls} showDetailsControls={showDetailsControls} moreButtons={moreButtons} - filtersOverride={filtersOverride} />
{ insight: QueryBasedInsightModel areDetailsShown?: boolean @@ -56,7 +55,6 @@ export function InsightMeta({ ribbonColor, dashboardId, updateColor, - filtersOverride, removeFromDashboard, deleteWithUndo, refresh, @@ -100,7 +98,7 @@ export function InsightMeta({ topHeading={} meta={ <> - +

{name || {summary}} {loading && ( @@ -132,7 +130,7 @@ export function InsightMeta({ moreButtons={ <> <> - + View {refresh && ( diff --git a/frontend/src/lib/components/DatabaseTableTree/TreeRow.tsx b/frontend/src/lib/components/DatabaseTableTree/TreeRow.tsx index 79661f2e5df3e..47a425e04243a 100644 --- a/frontend/src/lib/components/DatabaseTableTree/TreeRow.tsx +++ b/frontend/src/lib/components/DatabaseTableTree/TreeRow.tsx @@ -92,6 +92,9 @@ export function TreeFolderRow({ item, depth, onClick, selectedRow, dropdownOverl if (item.table.status === 'Failed') { return `Materialization failed` } + if (item.table.status === 'Modified') { + return `View definition modified since last materialization` + } if (item.table.status === 'Completed') { return `Last materialized ${humanFriendlyDetailedTime(item.table.last_run_at)}` } @@ -99,7 +102,7 @@ export function TreeFolderRow({ item, depth, onClick, selectedRow, dropdownOverl return '' } - const getIconColor = (): 'text-primary' | 'text-danger' | 'text-success' => { + const getIconColor = (): 'text-primary' | 'text-danger' | 'text-warning' | 'text-success' => { if (item.table?.type === 'materialized_view') { if (item.table.status === 'Running') { return 'text-primary' @@ -107,6 +110,9 @@ export function TreeFolderRow({ item, depth, onClick, selectedRow, dropdownOverl if (item.table.status === 'Failed') { return 'text-danger' } + if (item.table.status === 'Modified') { + return 'text-warning' + } } return 'text-success' } diff --git a/frontend/src/lib/components/PropertyFilters/utils.ts b/frontend/src/lib/components/PropertyFilters/utils.ts index be77a88c4211c..6eaafca785e5f 100644 --- a/frontend/src/lib/components/PropertyFilters/utils.ts +++ b/frontend/src/lib/components/PropertyFilters/utils.ts @@ -16,6 +16,7 @@ import { AnyPropertyFilter, CohortPropertyFilter, CohortType, + DataWarehousePersonPropertyFilter, DataWarehousePropertyFilter, ElementPropertyFilter, EmptyPropertyFilter, @@ -220,6 +221,11 @@ export function isGroupPropertyFilter(filter?: AnyFilterLike | null): filter is export function isDataWarehousePropertyFilter(filter?: AnyFilterLike | null): filter is DataWarehousePropertyFilter { return filter?.type === PropertyFilterType.DataWarehouse } +export function isDataWarehousePersonPropertyFilter( + filter?: AnyFilterLike | null +): filter is DataWarehousePropertyFilter { + return filter?.type === PropertyFilterType.DataWarehousePersonProperty +} export function isFeaturePropertyFilter(filter?: AnyFilterLike | null): filter is FeaturePropertyFilter { return filter?.type === PropertyFilterType.Feature } @@ -252,7 +258,8 @@ export function isPropertyFilterWithOperator( | LogEntryPropertyFilter | FeaturePropertyFilter | GroupPropertyFilter - | DataWarehousePropertyFilter { + | DataWarehousePropertyFilter + | DataWarehousePersonPropertyFilter { return ( !isPropertyGroupFilterLike(filter) && (isEventPropertyFilter(filter) || @@ -264,7 +271,8 @@ export function isPropertyFilterWithOperator( isFeaturePropertyFilter(filter) || isGroupPropertyFilter(filter) || isCohortPropertyFilter(filter) || - isDataWarehousePropertyFilter(filter)) + isDataWarehousePropertyFilter(filter) || + isDataWarehousePersonPropertyFilter(filter)) ) } diff --git a/frontend/src/lib/components/Support/supportLogic.ts b/frontend/src/lib/components/Support/supportLogic.ts index 1803fccee2751..75086da6c1cfe 100644 --- a/frontend/src/lib/components/Support/supportLogic.ts +++ b/frontend/src/lib/components/Support/supportLogic.ts @@ -141,7 +141,7 @@ export const TARGET_AREA_TO_NAME = [ { value: 'data_warehouse', 'data-attr': `support-form-target-area-data_warehouse`, - label: 'Data warehouse (beta)', + label: 'Data warehouse', }, { value: 'feature_flags', @@ -171,7 +171,7 @@ export const TARGET_AREA_TO_NAME = [ { value: 'web_analytics', 'data-attr': `support-form-target-area-web_analytics`, - label: 'Web Analytics (beta)', + label: 'Web Analytics', }, ], }, diff --git a/frontend/src/lib/constants.tsx b/frontend/src/lib/constants.tsx index 8c36526763d4f..1b73d2ac8e2bf 100644 --- a/frontend/src/lib/constants.tsx +++ b/frontend/src/lib/constants.tsx @@ -153,8 +153,6 @@ export const FEATURE_FLAGS = { HEDGEHOG_MODE_DEBUG: 'hedgehog-mode-debug', // owner: @benjackwhite HIGH_FREQUENCY_BATCH_EXPORTS: 'high-frequency-batch-exports', // owner: @tomasfarias PERSON_BATCH_EXPORTS: 'person-batch-exports', // owner: @tomasfarias - // owner: #team-replay, only to be enabled for PostHog team testing - EXCEPTION_AUTOCAPTURE: 'exception-autocapture', FF_DASHBOARD_TEMPLATES: 'ff-dashboard-templates', // owner: @EDsCODE ARTIFICIAL_HOG: 'artificial-hog', // owner: @Twixes CS_DASHBOARDS: 'cs-dashboards', // owner: @pauldambra diff --git a/frontend/src/lib/lemon-ui/LemonDialog/LemonDialog.tsx b/frontend/src/lib/lemon-ui/LemonDialog/LemonDialog.tsx index 4efcb5666664c..ca3b0a1cf2535 100644 --- a/frontend/src/lib/lemon-ui/LemonDialog/LemonDialog.tsx +++ b/frontend/src/lib/lemon-ui/LemonDialog/LemonDialog.tsx @@ -16,7 +16,7 @@ export type LemonFormDialogProps = LemonDialogFormPropsType & export type LemonDialogProps = Pick< LemonModalProps, - 'title' | 'description' | 'width' | 'maxWidth' | 'inline' | 'footer' + 'title' | 'description' | 'width' | 'maxWidth' | 'inline' | 'footer' | 'zIndex' > & { primaryButton?: LemonButtonProps | null secondaryButton?: LemonButtonProps | null diff --git a/frontend/src/lib/lemon-ui/LemonModal/LemonModal.scss b/frontend/src/lib/lemon-ui/LemonModal/LemonModal.scss index ee142ce426e21..44a076573a9b8 100644 --- a/frontend/src/lib/lemon-ui/LemonModal/LemonModal.scss +++ b/frontend/src/lib/lemon-ui/LemonModal/LemonModal.scss @@ -21,6 +21,12 @@ background-color: transparent; backdrop-filter: blur(0); } + + @each $i in (1061, 1062, 1066, 1067, 1068, 1069) { + &.LemonModal__overlay--z-#{$i} { + z-index: #{$i}; + } + } } .LemonModal { diff --git a/frontend/src/lib/lemon-ui/LemonModal/LemonModal.tsx b/frontend/src/lib/lemon-ui/LemonModal/LemonModal.tsx index 3a50ea89355f8..3b7683b48e7b4 100644 --- a/frontend/src/lib/lemon-ui/LemonModal/LemonModal.tsx +++ b/frontend/src/lib/lemon-ui/LemonModal/LemonModal.tsx @@ -46,6 +46,11 @@ export interface LemonModalProps { contentRef?: React.RefCallback overlayRef?: React.RefCallback 'data-attr'?: string + /** + * some components need more fine control of the z-index + * they can push a specific value to control their position in the stacking order + */ + zIndex?: '1061' | '1062' | '1066' | '1067' | '1068' | '1069' } export const LemonModalHeader = ({ children, className }: LemonModalInnerProps): JSX.Element => { @@ -84,6 +89,7 @@ export function LemonModal({ overlayRef, hideCloseButton = false, 'data-attr': dataAttr, + zIndex, }: LemonModalProps): JSX.Element { const nodeRef = useRef(null) const [ignoredOverlayClickCount, setIgnoredOverlayClickCount] = useState(0) @@ -157,7 +163,6 @@ export function LemonModal({ width = !fullScreen ? width : undefined maxWidth = !fullScreen ? maxWidth : undefined - const floatingContainer = useFloatingContainer() return inline ? ( @@ -184,6 +189,7 @@ export function LemonModal({ className={clsx('LemonModal', fullScreen && 'LemonModal--fullscreen')} overlayClassName={clsx( 'LemonModal__overlay', + zIndex && `LemonModal__overlay--z-${zIndex}`, forceAbovePopovers && 'LemonModal__overlay--force-modal-above-popovers' )} style={{ diff --git a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts index aebeaf774f973..742320e8197eb 100644 --- a/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts +++ b/frontend/src/queries/nodes/InsightQuery/utils/filtersToQueryNode.ts @@ -51,6 +51,7 @@ import { DataWarehouseFilter, FilterType, FunnelExclusionLegacy, + FunnelMathType, FunnelsFilterType, GroupMathType, HogQLMathType, @@ -84,7 +85,7 @@ const actorsOnlyMathTypes = [ HogQLMathType.HogQL, ] -const funnelsMathTypes = [BaseMathType.FirstTimeForUser] +const funnelsMathTypes = [FunnelMathType.FirstTimeForUser, FunnelMathType.FirstTimeForUserWithFilters] type FilterTypeActionsAndEvents = { events?: ActionFilter[] @@ -118,7 +119,7 @@ export const legacyEntityToNode = ( } if (mathAvailability !== MathAvailability.None) { - // only trends and stickiness insights support math. + // only trends, funnels, and stickiness insights support math. // transition to then default math for stickiness, when an unsupported math type is encountered. if (mathAvailability === MathAvailability.ActorsOnly && !actorsOnlyMathTypes.includes(entity.math as any)) { shared = { diff --git a/frontend/src/queries/query.ts b/frontend/src/queries/query.ts index 5a39c0902e9e5..1ea01c13868f1 100644 --- a/frontend/src/queries/query.ts +++ b/frontend/src/queries/query.ts @@ -147,10 +147,20 @@ export async function performQuery( logParams.clickhouse_sql = (response as HogQLQueryResponse)?.clickhouse } } - posthog.capture('query completed', { query: queryNode, duration: performance.now() - startTime, ...logParams }) + posthog.capture('query completed', { + query: queryNode, + queryId, + duration: performance.now() - startTime, + ...logParams, + }) return response } catch (e) { - posthog.capture('query failed', { query: queryNode, duration: performance.now() - startTime, ...logParams }) + posthog.capture('query failed', { + query: queryNode, + queryId, + duration: performance.now() - startTime, + ...logParams, + }) throw e } } diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index 7c1bfad6dc1b2..9f35a775afd6b 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -945,6 +945,71 @@ ], "type": "object" }, + "CachedEventTaxonomyQueryResponse": { + "additionalProperties": false, + "properties": { + "cache_key": { + "type": "string" + }, + "cache_target_age": { + "format": "date-time", + "type": "string" + }, + "calculation_trigger": { + "description": "What triggered the calculation of the query, leave empty if user/immediate", + "type": "string" + }, + "error": { + "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.", + "type": "string" + }, + "hogql": { + "description": "Generated HogQL query.", + "type": "string" + }, + "is_cached": { + "type": "boolean" + }, + "last_refresh": { + "format": "date-time", + "type": "string" + }, + "modifiers": { + "$ref": "#/definitions/HogQLQueryModifiers", + "description": "Modifiers used when performing the query" + }, + "next_allowed_client_refresh": { + "format": "date-time", + "type": "string" + }, + "query_status": { + "$ref": "#/definitions/QueryStatus", + "description": "Query status indicates whether next to the provided data, a query is still running." + }, + "results": { + "$ref": "#/definitions/EventTaxonomyResponse" + }, + "timezone": { + "type": "string" + }, + "timings": { + "description": "Measured timings for different parts of the query generation process", + "items": { + "$ref": "#/definitions/QueryTiming" + }, + "type": "array" + } + }, + "required": [ + "cache_key", + "is_cached", + "last_refresh", + "next_allowed_client_refresh", + "results", + "timezone" + ], + "type": "object" + }, "CachedEventsQueryResponse": { "additionalProperties": false, "properties": { @@ -1973,6 +2038,71 @@ ], "type": "object" }, + "CachedTeamTaxonomyQueryResponse": { + "additionalProperties": false, + "properties": { + "cache_key": { + "type": "string" + }, + "cache_target_age": { + "format": "date-time", + "type": "string" + }, + "calculation_trigger": { + "description": "What triggered the calculation of the query, leave empty if user/immediate", + "type": "string" + }, + "error": { + "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.", + "type": "string" + }, + "hogql": { + "description": "Generated HogQL query.", + "type": "string" + }, + "is_cached": { + "type": "boolean" + }, + "last_refresh": { + "format": "date-time", + "type": "string" + }, + "modifiers": { + "$ref": "#/definitions/HogQLQueryModifiers", + "description": "Modifiers used when performing the query" + }, + "next_allowed_client_refresh": { + "format": "date-time", + "type": "string" + }, + "query_status": { + "$ref": "#/definitions/QueryStatus", + "description": "Query status indicates whether next to the provided data, a query is still running." + }, + "results": { + "$ref": "#/definitions/TeamTaxonomyResponse" + }, + "timezone": { + "type": "string" + }, + "timings": { + "description": "Measured timings for different parts of the query generation process", + "items": { + "$ref": "#/definitions/QueryTiming" + }, + "type": "array" + } + }, + "required": [ + "cache_key", + "is_cached", + "last_refresh", + "next_allowed_client_refresh", + "results", + "timezone" + ], + "type": "object" + }, "CachedTrendsQueryResponse": { "additionalProperties": false, "properties": { @@ -4311,6 +4441,86 @@ "required": ["key", "operator", "type"], "type": "object" }, + "EventTaxonomyItem": { + "additionalProperties": false, + "properties": { + "property": { + "type": "string" + }, + "sample_count": { + "type": "integer" + }, + "sample_values": { + "items": { + "type": "string" + }, + "type": "array" + } + }, + "required": ["property", "sample_values", "sample_count"], + "type": "object" + }, + "EventTaxonomyQuery": { + "additionalProperties": false, + "properties": { + "event": { + "type": "string" + }, + "kind": { + "const": "EventTaxonomyQuery", + "type": "string" + }, + "modifiers": { + "$ref": "#/definitions/HogQLQueryModifiers", + "description": "Modifiers used when performing the query" + }, + "response": { + "$ref": "#/definitions/EventTaxonomyQueryResponse" + } + }, + "required": ["event", "kind"], + "type": "object" + }, + "EventTaxonomyQueryResponse": { + "additionalProperties": false, + "description": "All analytics query responses must inherit from this.", + "properties": { + "error": { + "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.", + "type": "string" + }, + "hogql": { + "description": "Generated HogQL query.", + "type": "string" + }, + "modifiers": { + "$ref": "#/definitions/HogQLQueryModifiers", + "description": "Modifiers used when performing the query" + }, + "query_status": { + "$ref": "#/definitions/QueryStatus", + "description": "Query status indicates whether next to the provided data, a query is still running." + }, + "results": { + "$ref": "#/definitions/EventTaxonomyResponse" + }, + "timings": { + "description": "Measured timings for different parts of the query generation process", + "items": { + "$ref": "#/definitions/QueryTiming" + }, + "type": "array" + } + }, + "required": ["results"], + "type": "object" + }, + "EventTaxonomyResponse": { + "items": { + "$ref": "#/definitions/EventTaxonomyItem" + }, + "type": "array" + }, "EventType": { "additionalProperties": false, "properties": { @@ -5163,6 +5373,10 @@ "enum": ["horizontal", "vertical"], "type": "string" }, + "FunnelMathType": { + "enum": ["total", "first_time_for_user", "first_time_for_user_with_filters"], + "type": "string" + }, "FunnelPathType": { "enum": ["funnel_path_before_step", "funnel_path_between_steps", "funnel_path_after_step"], "type": "string" @@ -6922,6 +7136,9 @@ { "$ref": "#/definitions/BaseMathType" }, + { + "$ref": "#/definitions/FunnelMathType" + }, { "$ref": "#/definitions/PropertyMathType" }, @@ -6992,7 +7209,9 @@ "WebGoalsQuery", "ExperimentFunnelQuery", "ExperimentTrendQuery", - "DatabaseSchemaQuery" + "DatabaseSchemaQuery", + "TeamTaxonomyQuery", + "EventTaxonomyQuery" ], "type": "string" }, @@ -10414,6 +10633,77 @@ ], "type": "string" }, + "TeamTaxonomyItem": { + "additionalProperties": false, + "properties": { + "count": { + "type": "integer" + }, + "event": { + "type": "string" + } + }, + "required": ["event", "count"], + "type": "object" + }, + "TeamTaxonomyQuery": { + "additionalProperties": false, + "properties": { + "kind": { + "const": "TeamTaxonomyQuery", + "type": "string" + }, + "modifiers": { + "$ref": "#/definitions/HogQLQueryModifiers", + "description": "Modifiers used when performing the query" + }, + "response": { + "$ref": "#/definitions/TeamTaxonomyQueryResponse" + } + }, + "required": ["kind"], + "type": "object" + }, + "TeamTaxonomyQueryResponse": { + "additionalProperties": false, + "description": "All analytics query responses must inherit from this.", + "properties": { + "error": { + "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.", + "type": "string" + }, + "hogql": { + "description": "Generated HogQL query.", + "type": "string" + }, + "modifiers": { + "$ref": "#/definitions/HogQLQueryModifiers", + "description": "Modifiers used when performing the query" + }, + "query_status": { + "$ref": "#/definitions/QueryStatus", + "description": "Query status indicates whether next to the provided data, a query is still running." + }, + "results": { + "$ref": "#/definitions/TeamTaxonomyResponse" + }, + "timings": { + "description": "Measured timings for different parts of the query generation process", + "items": { + "$ref": "#/definitions/QueryTiming" + }, + "type": "array" + } + }, + "required": ["results"], + "type": "object" + }, + "TeamTaxonomyResponse": { + "items": { + "$ref": "#/definitions/TeamTaxonomyItem" + }, + "type": "array" + }, "TestBasicQueryResponse": { "additionalProperties": false, "deprecated": "Only exported for use in test_query_runner.py! Don't use anywhere else.", diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts index 2e7c0598d8048..9bbffba136dab 100644 --- a/frontend/src/queries/schema.ts +++ b/frontend/src/queries/schema.ts @@ -17,6 +17,7 @@ import { FeaturePropertyFilter, FilterLogicalOperator, FilterType, + FunnelMathType, FunnelsFilterType, GroupMathType, GroupPropertyFilter, @@ -104,6 +105,10 @@ export enum NodeKind { // Database metadata DatabaseSchemaQuery = 'DatabaseSchemaQuery', + + // AI queries + TeamTaxonomyQuery = 'TeamTaxonomyQuery', + EventTaxonomyQuery = 'EventTaxonomyQuery', } export type AnyDataNode = @@ -456,7 +461,13 @@ export interface HogQLAutocomplete extends DataNode { endPosition: integer } -export type MathType = BaseMathType | PropertyMathType | CountPerActorMathType | GroupMathType | HogQLMathType +export type MathType = + | BaseMathType + | FunnelMathType + | PropertyMathType + | CountPerActorMathType + | GroupMathType + | HogQLMathType export interface EntityNode extends Node { name?: string @@ -1979,3 +1990,35 @@ export interface TrendsAlertConfig { export interface HogCompileResponse { bytecode: any[] } + +export interface TeamTaxonomyItem { + event: string + count: integer +} + +export type TeamTaxonomyResponse = TeamTaxonomyItem[] + +export interface TeamTaxonomyQuery extends DataNode { + kind: NodeKind.TeamTaxonomyQuery +} + +export type TeamTaxonomyQueryResponse = AnalyticsQueryResponseBase + +export type CachedTeamTaxonomyQueryResponse = CachedQueryResponse + +export interface EventTaxonomyItem { + property: string + sample_values: string[] + sample_count: integer +} + +export type EventTaxonomyResponse = EventTaxonomyItem[] + +export interface EventTaxonomyQuery extends DataNode { + kind: NodeKind.EventTaxonomyQuery + event: string +} + +export type EventTaxonomyQueryResponse = AnalyticsQueryResponseBase + +export type CachedEventTaxonomyQueryResponse = CachedQueryResponse diff --git a/frontend/src/scenes/actions/actionLogic.ts b/frontend/src/scenes/actions/actionLogic.ts index 2656b902e6352..15d95e1475da2 100644 --- a/frontend/src/scenes/actions/actionLogic.ts +++ b/frontend/src/scenes/actions/actionLogic.ts @@ -77,7 +77,7 @@ export const actionLogic = kea([ (action, inProgressName): Breadcrumb[] => [ { key: Scene.DataManagement, - name: `Data Management`, + name: `Data management`, path: urls.eventDefinitions(), }, { diff --git a/frontend/src/scenes/annotations/AnnotationModal.tsx b/frontend/src/scenes/annotations/AnnotationModal.tsx index 8f7450938f5e2..9e3a69a5e29a5 100644 --- a/frontend/src/scenes/annotations/AnnotationModal.tsx +++ b/frontend/src/scenes/annotations/AnnotationModal.tsx @@ -82,6 +82,7 @@ export function AnnotationModal({

} + width={512} >
diff --git a/frontend/src/scenes/billing/BillingProductAddon.tsx b/frontend/src/scenes/billing/BillingProductAddon.tsx index b490bfc255d20..df746aba9add9 100644 --- a/frontend/src/scenes/billing/BillingProductAddon.tsx +++ b/frontend/src/scenes/billing/BillingProductAddon.tsx @@ -71,7 +71,11 @@ export const BillingProductAddon = ({ addon }: { addon: BillingProductV2AddonTyp addon.plans?.find((plan) => plan.plan_key === 'addon-20240404-og-customers') return ( -
+
{getProductIcon(addon.name, addon.icon_key, 'text-2xl')}
diff --git a/frontend/src/scenes/dashboard/DashboardItems.tsx b/frontend/src/scenes/dashboard/DashboardItems.tsx index 55f4a1cf0438c..bef19ceff2796 100644 --- a/frontend/src/scenes/dashboard/DashboardItems.tsx +++ b/frontend/src/scenes/dashboard/DashboardItems.tsx @@ -27,7 +27,6 @@ export function DashboardItems(): JSX.Element { refreshStatus, canEditDashboard, itemsLoading, - temporaryFilters, } = useValues(dashboardLogic) const { updateLayouts, @@ -153,7 +152,6 @@ export function DashboardItems(): JSX.Element { showDetailsControls={placement != DashboardPlacement.Export} placement={placement} loadPriority={smLayout ? smLayout.y * 1000 + smLayout.x : undefined} - filtersOverride={temporaryFilters} {...commonTileProps} /> ) diff --git a/frontend/src/scenes/data-management/DataManagementScene.tsx b/frontend/src/scenes/data-management/DataManagementScene.tsx index 07661267bf164..bc0d6d444ef2f 100644 --- a/frontend/src/scenes/data-management/DataManagementScene.tsx +++ b/frontend/src/scenes/data-management/DataManagementScene.tsx @@ -119,7 +119,7 @@ const dataManagementSceneLogic = kea([ return [ { key: Scene.DataManagement, - name: `Data Management`, + name: `Data management`, path: tabs.events.url, }, { diff --git a/frontend/src/scenes/data-management/definition/definitionLogic.ts b/frontend/src/scenes/data-management/definition/definitionLogic.ts index dfcbfa0a2d8f6..8a814720b6d74 100644 --- a/frontend/src/scenes/data-management/definition/definitionLogic.ts +++ b/frontend/src/scenes/data-management/definition/definitionLogic.ts @@ -113,7 +113,7 @@ export const definitionLogic = kea([ return [ { key: Scene.DataManagement, - name: `Data Management`, + name: `Data management`, path: isEvent ? urls.eventDefinitions() : urls.propertyDefinitions(), }, { diff --git a/frontend/src/scenes/data-warehouse/external/DataWarehouseTables.tsx b/frontend/src/scenes/data-warehouse/external/DataWarehouseTables.tsx index 69b36f2557e10..03b4da5e3a1c4 100644 --- a/frontend/src/scenes/data-warehouse/external/DataWarehouseTables.tsx +++ b/frontend/src/scenes/data-warehouse/external/DataWarehouseTables.tsx @@ -131,7 +131,7 @@ export const DatabaseTableTreeWithItems = ({ inline }: DatabaseTableTreeProps): > Add join - {table.type == 'view' && ( + {(table.type == 'view' || table.type == 'materialized_view') && ( { router.actions.push(urls.dataWarehouseView(table.id)) diff --git a/frontend/src/scenes/data-warehouse/external/dataWarehouseExternalSceneLogic.ts b/frontend/src/scenes/data-warehouse/external/dataWarehouseExternalSceneLogic.ts index 10fdb4cc1fd61..17da8d174d375 100644 --- a/frontend/src/scenes/data-warehouse/external/dataWarehouseExternalSceneLogic.ts +++ b/frontend/src/scenes/data-warehouse/external/dataWarehouseExternalSceneLogic.ts @@ -102,6 +102,8 @@ export const dataWarehouseExternalSceneLogic = kea { return (
-
- +
+ diff --git a/frontend/src/scenes/error-tracking/queries.test.ts b/frontend/src/scenes/error-tracking/queries.test.ts index 9f05cc21e472e..406a5837f4b10 100644 --- a/frontend/src/scenes/error-tracking/queries.test.ts +++ b/frontend/src/scenes/error-tracking/queries.test.ts @@ -1,4 +1,9 @@ -import { generateSparklineProps, parseSparklineSelection, SPARKLINE_CONFIGURATIONS } from './queries' +import { + generateSparklineProps, + parseSparklineSelection, + SPARKLINE_CONFIGURATIONS, + stringifyFingerprints, +} from './queries' describe('generateSparklineProps', () => { beforeAll(() => { @@ -131,3 +136,23 @@ describe('parseSparklineSelection', () => { expect(parseSparklineSelection('6w')).toEqual({ value: 6, displayAs: 'week' }) }) }) + +describe('stringifyFingerprints', () => { + it('works for basic case', async () => { + expect(stringifyFingerprints([['a', 'b', 'c']])).toEqual("[['a','b','c']]") + expect(stringifyFingerprints([['a']])).toEqual("[['a']]") + expect(stringifyFingerprints([])).toEqual('[]') + }) + + it('escapes single quotes correctly', async () => { + expect(stringifyFingerprints([["a'"]])).toEqual("[['a\\'']]") + expect(stringifyFingerprints([["a'", "b'"]])).toEqual("[['a\\'','b\\'']]") + expect(stringifyFingerprints([["a'", "b'"], ["c'"]])).toEqual("[['a\\'','b\\''],['c\\'']]") + }) + + it('escapes double quotes correctly', async () => { + expect(stringifyFingerprints([['a"']])).toEqual("[['a\"']]") + expect(stringifyFingerprints([['a"', 'b"']])).toEqual("[['a\"','b\"']]") + expect(stringifyFingerprints([['a"', 'b"'], ['c"']])).toEqual("[['a\"','b\"'],['c\"']]") + }) +}) diff --git a/frontend/src/scenes/error-tracking/queries.ts b/frontend/src/scenes/error-tracking/queries.ts index b506b5d40e310..0358e9096fd6d 100644 --- a/frontend/src/scenes/error-tracking/queries.ts +++ b/frontend/src/scenes/error-tracking/queries.ts @@ -184,12 +184,13 @@ export const errorTrackingGroupEventsQuery = ({ } // JSON.stringify wraps strings in double quotes and HogQL only supports single quote strings -const stringifyFingerprints = (fingerprints: ErrorTrackingGroup['fingerprint'][]): string => { - const stringifiedFingerprints = fingerprints.map((fp) => { - const stringifiedParts = fp.map((s) => `'${s}'`) - return `[${stringifiedParts.join(',')}]` - }) - return `[${stringifiedFingerprints.join(',')}]` +export const stringifyFingerprints = (fingerprints: ErrorTrackingGroup['fingerprint'][]): string => { + // so we escape all single quoted strings and replace double quotes with single quotes, unless they're already escaped. + // Also replace escaped double quotes with regular double quotes - this isn't valid JSON, but we aren't trying to generate JSON so its ok. + return JSON.stringify(fingerprints) + .replace(/'/g, "\\'") + .replace(/(? = [ { @@ -56,13 +58,15 @@ export function DistributionTable(): JSX.Element {
- } + onClick={() => openSidePanel(SidePanelTab.ExperimentFeatureFlag)} + type="secondary" + size="xsmall" className="font-semibold" - to={experiment.feature_flag ? urls.featureFlag(experiment.feature_flag.id) : undefined} > Manage distribution - +
diff --git a/frontend/src/scenes/experiments/ExperimentView/ReleaseConditionsTable.tsx b/frontend/src/scenes/experiments/ExperimentView/ReleaseConditionsTable.tsx index c0a4024e559f6..0f31b8929bf8b 100644 --- a/frontend/src/scenes/experiments/ExperimentView/ReleaseConditionsTable.tsx +++ b/frontend/src/scenes/experiments/ExperimentView/ReleaseConditionsTable.tsx @@ -1,17 +1,19 @@ import '../Experiment.scss' -import { LemonTable, LemonTableColumns, LemonTag, Link } from '@posthog/lemon-ui' -import { useValues } from 'kea' -import { urls } from 'scenes/urls' +import { IconFlag } from '@posthog/icons' +import { LemonButton, LemonTable, LemonTableColumns, LemonTag } from '@posthog/lemon-ui' +import { useActions, useValues } from 'kea' +import { sidePanelStateLogic } from '~/layout/navigation-3000/sidepanel/sidePanelStateLogic' import { groupsModel } from '~/models/groupsModel' -import { FeatureFlagGroupType } from '~/types' +import { FeatureFlagGroupType, SidePanelTab } from '~/types' import { experimentLogic } from '../experimentLogic' export function ReleaseConditionsTable(): JSX.Element { const { experiment } = useValues(experimentLogic) const { aggregationLabel } = useValues(groupsModel) + const { openSidePanel } = useActions(sidePanelStateLogic) const columns: LemonTableColumns = [ { @@ -61,13 +63,15 @@ export function ReleaseConditionsTable(): JSX.Element {
- } + onClick={() => openSidePanel(SidePanelTab.ExperimentFeatureFlag)} + type="secondary" + size="xsmall" className="font-semibold" - to={experiment.feature_flag ? urls.featureFlag(experiment.feature_flag.id) : undefined} > Manage release conditions - +
diff --git a/frontend/src/scenes/feature-flags/FeatureFlag.tsx b/frontend/src/scenes/feature-flags/FeatureFlag.tsx index 737724ae7ebb8..4e219519fd837 100644 --- a/frontend/src/scenes/feature-flags/FeatureFlag.tsx +++ b/frontend/src/scenes/feature-flags/FeatureFlag.tsx @@ -783,6 +783,11 @@ function FeatureFlagRollout({ readOnly }: { readOnly?: boolean }): JSX.Element { }) }} label="Enabled" + disabledReason={ + !featureFlag.can_edit + ? "You only have view access to this feature flag. To make changes, contact the flag's creator." + : null + } checked={featureFlag.active} /> Type diff --git a/frontend/src/scenes/feature-flags/featureFlagLogic.ts b/frontend/src/scenes/feature-flags/featureFlagLogic.ts index 956dc6fb98911..2f2933cf9335e 100644 --- a/frontend/src/scenes/feature-flags/featureFlagLogic.ts +++ b/frontend/src/scenes/feature-flags/featureFlagLogic.ts @@ -12,6 +12,7 @@ import { eventUsageLogic } from 'lib/utils/eventUsageLogic' import { dashboardsLogic } from 'scenes/dashboard/dashboards/dashboardsLogic' import { newDashboardLogic } from 'scenes/dashboard/newDashboardLogic' import { NEW_EARLY_ACCESS_FEATURE } from 'scenes/early-access-features/earlyAccessFeatureLogic' +import { experimentLogic } from 'scenes/experiments/experimentLogic' import { featureFlagsLogic, FeatureFlagsTab } from 'scenes/feature-flags/featureFlagsLogic' import { filterTrendsClientSideParams } from 'scenes/insights/sharedUtils' import { cleanFilters } from 'scenes/insights/utils/cleanFilters' @@ -20,6 +21,7 @@ import { NEW_SURVEY, NewSurvey } from 'scenes/surveys/constants' import { urls } from 'scenes/urls' import { userLogic } from 'scenes/userLogic' +import { sidePanelStateLogic } from '~/layout/navigation-3000/sidepanel/sidePanelStateLogic' import { groupsModel } from '~/models/groupsModel' import { getQueryBasedInsightModel } from '~/queries/nodes/InsightViz/utils' import { @@ -200,6 +202,8 @@ export const featureFlagLogic = kea([ ['submitNewDashboardSuccessWithResult'], featureFlagsLogic, ['updateFlag', 'deleteFlag'], + sidePanelStateLogic, + ['closeSidePanel'], ], })), actions({ @@ -465,6 +469,33 @@ export const featureFlagLogic = kea([ const preparedFlag = indexToVariantKeyFeatureFlagPayloads(flag) + try { + let savedFlag: FeatureFlagType + if (!updatedFlag.id) { + savedFlag = await api.create(`api/projects/${values.currentTeamId}/feature_flags`, preparedFlag) + if (values.roleBasedAccessEnabled && savedFlag.id) { + featureFlagPermissionsLogic({ flagId: null })?.actions.addAssociatedRoles(savedFlag.id) + } + } else { + savedFlag = await api.update( + `api/projects/${values.currentTeamId}/feature_flags/${updatedFlag.id}`, + preparedFlag + ) + } + + return variantKeyToIndexFeatureFlagPayloads(savedFlag) + } catch (error: any) { + if (error.code === 'behavioral_cohort_found' || error.code === 'cohort_does_not_exist') { + eventUsageLogic.actions.reportFailedToCreateFeatureFlagWithCohort(error.code, error.detail) + } + throw error + } + }, + saveSidebarExperimentFeatureFlag: async (updatedFlag: Partial) => { + const { created_at, id, ...flag } = updatedFlag + + const preparedFlag = indexToVariantKeyFeatureFlagPayloads(flag) + try { let savedFlag: FeatureFlagType if (!updatedFlag.id) { @@ -665,6 +696,19 @@ export const featureFlagLogic = kea([ featureFlag.id && router.actions.replace(urls.featureFlag(featureFlag.id)) actions.editFeatureFlag(false) }, + saveSidebarExperimentFeatureFlagSuccess: ({ featureFlag }) => { + lemonToast.success('Release conditions updated') + actions.updateFlag(featureFlag) + actions.editFeatureFlag(false) + actions.closeSidePanel() + + const currentPath = router.values.currentLocation.pathname + const experimentId = currentPath.split('/').pop() + + if (experimentId) { + experimentLogic({ experimentId: parseInt(experimentId) }).actions.loadExperiment() + } + }, deleteFeatureFlag: async ({ featureFlag }) => { await deleteWithUndo({ endpoint: `projects/${values.currentTeamId}/feature_flags`, diff --git a/frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx b/frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx index 0ce194ebca5c0..6340b62226ad7 100644 --- a/frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx +++ b/frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx @@ -6,7 +6,6 @@ import { CSS } from '@dnd-kit/utilities' import { IconCopy, IconEllipsis, IconFilter, IconPencil, IconTrash, IconWarning } from '@posthog/icons' import { LemonBadge, - LemonCheckbox, LemonDivider, LemonMenu, LemonSelect, @@ -21,8 +20,7 @@ import { PropertyKeyInfo } from 'lib/components/PropertyKeyInfo' import { SeriesGlyph, SeriesLetter } from 'lib/components/SeriesGlyph' import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types' import { TaxonomicPopover, TaxonomicStringPopover } from 'lib/components/TaxonomicPopover/TaxonomicPopover' -import { IconWithCount } from 'lib/lemon-ui/icons' -import { SortableDragIcon } from 'lib/lemon-ui/icons' +import { IconWithCount, SortableDragIcon } from 'lib/lemon-ui/icons' import { LemonButton } from 'lib/lemon-ui/LemonButton' import { LemonDropdown } from 'lib/lemon-ui/LemonDropdown' import { Tooltip } from 'lib/lemon-ui/Tooltip' @@ -529,20 +527,15 @@ export function ActionFilterRow({ { label: () => ( <> - { - onMathSelect( - index, - checked - ? BaseMathType.FirstTimeForUser - : undefined - ) - }} - data-attr={`math-first-time-for-user-${index}`} - label="Count by first time for user" - fullWidth + @@ -570,7 +563,7 @@ export function ActionFilterRow({
@@ -649,8 +642,13 @@ function useMathSelectorOptions({ const isStickiness = query && isInsightVizNode(query) && isStickinessQuery(query.source) - const { needsUpgradeForGroups, canStartUsingGroups, staticMathDefinitions, staticActorsOnlyMathDefinitions } = - useValues(mathsLogic) + const { + needsUpgradeForGroups, + canStartUsingGroups, + staticMathDefinitions, + funnelMathDefinitions, + staticActorsOnlyMathDefinitions, + } = useValues(mathsLogic) const [propertyMathTypeShown, setPropertyMathTypeShown] = useState( isPropertyValueMath(math) ? math : PropertyMathType.Average @@ -659,9 +657,14 @@ function useMathSelectorOptions({ isCountPerActorMath(math) ? math : CountPerActorMathType.Average ) - const options: LemonSelectOption[] = Object.entries( - mathAvailability != MathAvailability.ActorsOnly ? staticMathDefinitions : staticActorsOnlyMathDefinitions - ) + let definitions = staticMathDefinitions + if (mathAvailability === MathAvailability.FunnelsOnly) { + definitions = funnelMathDefinitions + } else if (mathAvailability === MathAvailability.ActorsOnly) { + definitions = staticActorsOnlyMathDefinitions + } + + const options: LemonSelectOption[] = Object.entries(definitions) .filter(([key]) => { if (isStickiness) { // Remove WAU and MAU from stickiness insights @@ -691,7 +694,7 @@ function useMathSelectorOptions({ } }) - if (mathAvailability !== MathAvailability.ActorsOnly) { + if (mathAvailability !== MathAvailability.ActorsOnly && mathAvailability !== MathAvailability.FunnelsOnly) { options.splice(1, 0, { value: countPerActorMathTypeShown, label: `Count per user ${COUNT_PER_ACTOR_MATH_DEFINITIONS[countPerActorMathTypeShown].shortName}`, @@ -749,12 +752,14 @@ function useMathSelectorOptions({ }) } - options.push({ - value: HogQLMathType.HogQL, - label: 'HogQL expression', - tooltip: 'Aggregate events by custom SQL expression.', - 'data-attr': `math-node-hogql-expression-${index}`, - }) + if (mathAvailability !== MathAvailability.FunnelsOnly) { + options.push({ + value: HogQLMathType.HogQL, + label: 'HogQL expression', + tooltip: 'Aggregate events by custom SQL expression.', + 'data-attr': `math-node-hogql-expression-${index}`, + }) + } return [ { diff --git a/frontend/src/scenes/insights/insightSceneLogic.tsx b/frontend/src/scenes/insights/insightSceneLogic.tsx index 8de7a8caa2c67..e9b39ef503b59 100644 --- a/frontend/src/scenes/insights/insightSceneLogic.tsx +++ b/frontend/src/scenes/insights/insightSceneLogic.tsx @@ -4,6 +4,7 @@ import { CombinedLocation } from 'kea-router/lib/utils' import { objectsEqual } from 'kea-test-utils' import { AlertType } from 'lib/components/Alerts/types' import { eventUsageLogic, InsightEventSource } from 'lib/utils/eventUsageLogic' +import { dashboardLogic } from 'scenes/dashboard/dashboardLogic' import { createEmptyInsight, insightLogic } from 'scenes/insights/insightLogic' import { insightLogicType } from 'scenes/insights/insightLogicType' import { preflightLogic } from 'scenes/PreflightCheck/preflightLogic' @@ -19,7 +20,7 @@ import { cohortsModel } from '~/models/cohortsModel' import { groupsModel } from '~/models/groupsModel' import { getDefaultQuery } from '~/queries/nodes/InsightViz/utils' import { DashboardFilter, Node } from '~/queries/schema' -import { ActivityScope, Breadcrumb, InsightShortId, InsightType, ItemMode } from '~/types' +import { ActivityScope, Breadcrumb, DashboardType, InsightShortId, InsightType, ItemMode } from '~/types' import { insightDataLogic } from './insightDataLogic' import { insightDataLogicType } from './insightDataLogicType' @@ -49,12 +50,16 @@ export const insightSceneLogic = kea([ insightMode: ItemMode, itemId: string | undefined, alertId: AlertType['id'] | undefined, + dashboardId: DashboardType['id'] | undefined, + dashboardName: DashboardType['name'] | undefined, filtersOverride: DashboardFilter | undefined ) => ({ insightId, insightMode, itemId, alertId, + dashboardId, + dashboardName, filtersOverride, }), setInsightLogicRef: (logic: BuiltLogic | null, unmount: null | (() => void)) => ({ @@ -99,6 +104,18 @@ export const insightSceneLogic = kea([ setSceneState: (_, { alertId }) => (alertId !== undefined ? alertId : null), }, ], + dashboardId: [ + null as null | DashboardType['id'], + { + setSceneState: (_, { dashboardId }) => (dashboardId !== undefined ? dashboardId : null), + }, + ], + dashboardName: [ + null as null | DashboardType['name'], + { + setSceneState: (_, { dashboardName }) => (dashboardName !== undefined ? dashboardName : null), + }, + ], filtersOverride: [ null as null | DashboardFilter, { @@ -132,17 +149,42 @@ export const insightSceneLogic = kea([ (s) => [ s.insightLogicRef, s.insight, + s.dashboardId, + s.dashboardName, groupsModel.selectors.aggregationLabel, cohortsModel.selectors.cohortsById, mathsLogic.selectors.mathDefinitions, ], - (insightLogicRef, insight, aggregationLabel, cohortsById, mathDefinitions): Breadcrumb[] => { + ( + insightLogicRef, + insight, + dashboardId, + dashboardName, + aggregationLabel, + cohortsById, + mathDefinitions + ): Breadcrumb[] => { return [ - { - key: Scene.SavedInsights, - name: 'Product analytics', - path: urls.savedInsights(), - }, + ...(dashboardId !== null && dashboardName + ? [ + { + key: Scene.Dashboards, + name: 'Dashboards', + path: urls.dashboards(), + }, + { + key: Scene.Dashboard, + name: dashboardName, + path: urls.dashboard(dashboardId), + }, + ] + : [ + { + key: Scene.SavedInsights, + name: 'Product analytics', + path: urls.savedInsights(), + }, + ]), { key: [Scene.Insight, insight?.short_id || 'new'], name: @@ -244,14 +286,27 @@ export const insightSceneLogic = kea([ return } + const dashboardName = dashboardLogic.findMounted({ id: dashboard })?.values.dashboard?.name + const filtersOverride = dashboardLogic.findMounted({ id: dashboard })?.values.temporaryFilters + if ( insightId !== values.insightId || insightMode !== values.insightMode || itemId !== values.itemId || alert_id !== values.alertId || - !objectsEqual(searchParams['filters_override'], values.filtersOverride) + dashboard !== values.dashboardId || + dashboardName !== values.dashboardName || + !objectsEqual(filtersOverride, values.filtersOverride) ) { - actions.setSceneState(insightId, insightMode, itemId, alert_id, searchParams['filters_override']) + actions.setSceneState( + insightId, + insightMode, + itemId, + alert_id, + dashboard, + dashboardName, + filtersOverride + ) } let queryFromUrl: Node | null = null diff --git a/frontend/src/scenes/max/Thread.tsx b/frontend/src/scenes/max/Thread.tsx index df081358aa3b9..362c2a74b473f 100644 --- a/frontend/src/scenes/max/Thread.tsx +++ b/frontend/src/scenes/max/Thread.tsx @@ -1,11 +1,12 @@ import { LemonButton, Spinner } from '@posthog/lemon-ui' import clsx from 'clsx' import { useValues } from 'kea' +import { IconOpenInNew } from 'lib/lemon-ui/icons' import React from 'react' +import { urls } from 'scenes/urls' -import { queryNodeToFilter } from '~/queries/nodes/InsightQuery/utils/queryNodeToFilter' import { Query } from '~/queries/Query/Query' -import { NodeKind } from '~/queries/schema' +import { InsightQueryNode, InsightVizNode, NodeKind } from '~/queries/schema' import { maxLogic } from './maxLogic' @@ -51,12 +52,14 @@ export function Thread(): JSX.Element | null { } targetBlank > - Edit Query + Open as new insight )} diff --git a/frontend/src/scenes/persons-management/personsManagementSceneLogic.tsx b/frontend/src/scenes/persons-management/personsManagementSceneLogic.tsx index fc7654dad7c55..c96a9718057e5 100644 --- a/frontend/src/scenes/persons-management/personsManagementSceneLogic.tsx +++ b/frontend/src/scenes/persons-management/personsManagementSceneLogic.tsx @@ -52,7 +52,7 @@ export const personsManagementSceneLogic = kea( { key: 'persons', url: urls.persons(), - label: 'People', + label: 'People & groups', content: , }, { diff --git a/frontend/src/scenes/pipeline/batch-exports/BatchExportEditForm.tsx b/frontend/src/scenes/pipeline/batch-exports/BatchExportEditForm.tsx index 24cf9f3eba7c9..830749130cf65 100644 --- a/frontend/src/scenes/pipeline/batch-exports/BatchExportEditForm.tsx +++ b/frontend/src/scenes/pipeline/batch-exports/BatchExportEditForm.tsx @@ -165,6 +165,12 @@ export function BatchExportsEditFields({ { value: 'eu-north-1', label: 'Europe (Stockholm)' }, { value: 'me-south-1', label: 'Middle East (Bahrain)' }, { value: 'sa-east-1', label: 'South America (São Paulo)' }, + { value: 'auto', label: 'Automatic (AUTO)' }, + { value: 'apac', label: 'Asia Pacific (APAC)' }, + { value: 'eeur', label: 'Eastern Europe (EEUR)' }, + { value: 'enam', label: 'Eastern North America (ENAM)' }, + { value: 'weur', label: 'Western Europe (WEUR)' }, + { value: 'wnam', label: 'Western North America (WNAM)' }, ]} /> @@ -405,7 +411,6 @@ export function BatchExportsEditFields({ { value: 'varchar', label: 'VARCHAR(65535)' }, { value: 'super', label: 'SUPER' }, ]} - value="varchar" /> diff --git a/frontend/src/scenes/pipeline/batchExportRunsLogic.tsx b/frontend/src/scenes/pipeline/batchExportRunsLogic.tsx index 8687c7756338b..a77914fe4cc4e 100644 --- a/frontend/src/scenes/pipeline/batchExportRunsLogic.tsx +++ b/frontend/src/scenes/pipeline/batchExportRunsLogic.tsx @@ -4,7 +4,6 @@ import { forms } from 'kea-forms' import { loaders } from 'kea-loaders' import api, { PaginatedResponse } from 'lib/api' import { Dayjs, dayjs } from 'lib/dayjs' -import { dayjsUtcToTimezone } from 'lib/dayjs' import { teamLogic } from 'scenes/teamLogic' import { BatchExportRun, GroupedBatchExportRuns, RawBatchExportRun } from '~/types' @@ -184,12 +183,10 @@ export const batchExportRunsLogic = kea([ return runs.map((run) => { return { ...run, - created_at: dayjsUtcToTimezone(run.created_at, teamLogic.values.timezone), - data_interval_start: dayjsUtcToTimezone(run.data_interval_start, teamLogic.values.timezone), - data_interval_end: dayjsUtcToTimezone(run.data_interval_end, teamLogic.values.timezone), - last_updated_at: run.last_updated_at - ? dayjsUtcToTimezone(run.last_updated_at, teamLogic.values.timezone) - : undefined, + created_at: dayjs(run.created_at), + data_interval_start: dayjs(run.data_interval_start), + data_interval_end: dayjs(run.data_interval_end), + last_updated_at: run.last_updated_at ? dayjs(run.last_updated_at) : undefined, } }) }, @@ -209,21 +206,19 @@ export const batchExportRunsLogic = kea([ const key = `${run.data_interval_start}-${run.data_interval_end}` if (!groupedRuns[key]) { groupedRuns[key] = { - data_interval_start: dayjsUtcToTimezone(run.data_interval_start, teamLogic.values.timezone), - data_interval_end: dayjsUtcToTimezone(run.data_interval_end, teamLogic.values.timezone), + data_interval_start: dayjs(run.data_interval_start), + data_interval_end: dayjs(run.data_interval_end), runs: [], - last_run_at: dayjsUtcToTimezone(run.created_at, teamLogic.values.timezone), + last_run_at: dayjs(run.created_at), } } groupedRuns[key].runs.push({ ...run, - created_at: dayjsUtcToTimezone(run.created_at, teamLogic.values.timezone), - data_interval_start: dayjsUtcToTimezone(run.data_interval_start, teamLogic.values.timezone), - data_interval_end: dayjsUtcToTimezone(run.data_interval_end, teamLogic.values.timezone), - last_updated_at: run.last_updated_at - ? dayjsUtcToTimezone(run.last_updated_at, teamLogic.values.timezone) - : undefined, + created_at: dayjs(run.created_at), + data_interval_start: dayjs(run.data_interval_start), + data_interval_end: dayjs(run.data_interval_end), + last_updated_at: run.last_updated_at ? dayjs(run.last_updated_at) : undefined, }) groupedRuns[key].runs.sort((a, b) => b.created_at.diff(a.created_at)) groupedRuns[key].last_run_at = groupedRuns[key].runs[0].created_at diff --git a/frontend/src/scenes/pipeline/destinations/DestinationsFilters.tsx b/frontend/src/scenes/pipeline/destinations/DestinationsFilters.tsx index c5cb47b403524..b5dd7704b519e 100644 --- a/frontend/src/scenes/pipeline/destinations/DestinationsFilters.tsx +++ b/frontend/src/scenes/pipeline/destinations/DestinationsFilters.tsx @@ -15,7 +15,7 @@ export function DestinationsFilters({ hideShowPaused, hideKind, }: DestinationsFiltersProps): JSX.Element | null { - const { user, filters } = useValues(destinationsFiltersLogic) + const { filters } = useValues(destinationsFiltersLogic) const { setFilters, openFeedbackDialog } = useActions(destinationsFiltersLogic) return ( @@ -42,16 +42,6 @@ export function DestinationsFilters({ onChange={(e) => setFilters({ showPaused: e ?? undefined })} /> )} - {(user?.is_staff || user?.is_impersonated) && ( - setFilters({ showHidden: e ?? undefined })} - /> - )} - {!hideKind && ( ([ diff --git a/frontend/src/scenes/pipeline/destinations/destinationsLogic.tsx b/frontend/src/scenes/pipeline/destinations/destinationsLogic.tsx index ffaadb8737208..38fa9a80270a9 100644 --- a/frontend/src/scenes/pipeline/destinations/destinationsLogic.tsx +++ b/frontend/src/scenes/pipeline/destinations/destinationsLogic.tsx @@ -161,7 +161,7 @@ export const pipelineDestinationsLogic = kea([ }, ], - _hogFunctions: [ + hogFunctions: [ [] as HogFunctionType[], { loadHogFunctions: async () => { @@ -205,11 +205,6 @@ export const pipelineDestinationsLogic = kea([ ], })), selectors({ - hogFunctions: [ - (s) => [s._hogFunctions, s.filters], - (hogFunctions, filters) => - filters.showHidden ? hogFunctions : hogFunctions.filter((hf) => !hf.name.includes('[CDP-TEST-HIDDEN]')), - ], paidHogFunctions: [ (s) => [s.hogFunctions], (hogFunctions) => { @@ -220,7 +215,7 @@ export const pipelineDestinationsLogic = kea([ }, ], loading: [ - (s) => [s.pluginsLoading, s.pluginConfigsLoading, s.batchExportConfigsLoading, s._hogFunctionsLoading], + (s) => [s.pluginsLoading, s.pluginConfigsLoading, s.batchExportConfigsLoading, s.hogFunctionsLoading], (pluginsLoading, pluginConfigsLoading, batchExportConfigsLoading, hogFunctionsLoading) => pluginsLoading || pluginConfigsLoading || batchExportConfigsLoading || hogFunctionsLoading, ], diff --git a/frontend/src/scenes/pipeline/hogfunctions/list/HogFunctionsList.tsx b/frontend/src/scenes/pipeline/hogfunctions/list/HogFunctionsList.tsx index 000b72e199d28..25a59059e6586 100644 --- a/frontend/src/scenes/pipeline/hogfunctions/list/HogFunctionsList.tsx +++ b/frontend/src/scenes/pipeline/hogfunctions/list/HogFunctionsList.tsx @@ -17,7 +17,7 @@ export function HogFunctionList({ extraControls, ...props }: HogFunctionListLogicProps & { extraControls?: JSX.Element }): JSX.Element { - const { user, loading, filteredHogFunctions, filters, hogFunctions, canEnableHogFunction } = useValues( + const { loading, filteredHogFunctions, filters, hogFunctions, canEnableHogFunction } = useValues( hogFunctionListLogic(props) ) const { loadHogFunctions, setFilters, resetFilters, toggleEnabled, deleteHogFunction } = useActions( @@ -47,15 +47,6 @@ export function HogFunctionList({ onChange={(e) => setFilters({ showPaused: e ?? undefined })} /> )} - {(user?.is_staff || user?.is_impersonated) && typeof props.forceFilters?.showHidden !== 'boolean' && ( - setFilters({ showHidden: e ?? undefined })} - /> - )} {extraControls}
diff --git a/frontend/src/scenes/pipeline/hogfunctions/list/hogFunctionListLogic.tsx b/frontend/src/scenes/pipeline/hogfunctions/list/hogFunctionListLogic.tsx index acd2cb2ba34ff..94a5d8274e6cb 100644 --- a/frontend/src/scenes/pipeline/hogfunctions/list/hogFunctionListLogic.tsx +++ b/frontend/src/scenes/pipeline/hogfunctions/list/hogFunctionListLogic.tsx @@ -21,7 +21,6 @@ export interface Fuse extends FuseClass {} export type HogFunctionListFilters = { search?: string showPaused?: boolean - showHidden?: boolean filters?: Record } @@ -70,7 +69,7 @@ export const hogFunctionListLogic = kea([ ], })), loaders(({ values, actions }) => ({ - _hogFunctions: [ + hogFunctions: [ [] as HogFunctionType[], { loadHogFunctions: async () => { @@ -114,18 +113,13 @@ export const hogFunctionListLogic = kea([ ] }, addHogFunction: ({ hogFunction }) => { - return [hogFunction, ...values._hogFunctions] + return [hogFunction, ...values.hogFunctions] }, }, ], })), selectors({ - loading: [(s) => [s._hogFunctionsLoading], (hogFunctionsLoading) => hogFunctionsLoading], - hogFunctions: [ - (s) => [s._hogFunctions, s.filters], - (hogFunctions, filters) => - filters.showHidden ? hogFunctions : hogFunctions.filter((hf) => !hf.name.includes('[CDP-TEST-HIDDEN]')), - ], + loading: [(s) => [s.hogFunctionsLoading], (hogFunctionsLoading) => hogFunctionsLoading], sortedHogFunctions: [ (s) => [s.hogFunctions], (hogFunctions): HogFunctionType[] => { diff --git a/frontend/src/scenes/saved-insights/SavedInsights.tsx b/frontend/src/scenes/saved-insights/SavedInsights.tsx index 58cf759339729..c09c8c7f3601a 100644 --- a/frontend/src/scenes/saved-insights/SavedInsights.tsx +++ b/frontend/src/scenes/saved-insights/SavedInsights.tsx @@ -350,6 +350,18 @@ export const QUERY_TYPES_METADATA: Record = { icon: IconFlask, inMenu: false, }, + [NodeKind.TeamTaxonomyQuery]: { + name: 'Team Taxonomy', + description: 'View the event taxonomy of the team', + icon: IconHogQL, + inMenu: false, + }, + [NodeKind.EventTaxonomyQuery]: { + name: 'Event Taxonomy', + description: 'View the event’s taxonomy', + icon: IconHogQL, + inMenu: false, + }, } export const INSIGHT_TYPE_OPTIONS: LemonSelectOptions = [ diff --git a/frontend/src/scenes/session-recordings/player/modal/SessionPlayerModal.tsx b/frontend/src/scenes/session-recordings/player/modal/SessionPlayerModal.tsx index c4df5222b253f..a7de38bbf4b5b 100644 --- a/frontend/src/scenes/session-recordings/player/modal/SessionPlayerModal.tsx +++ b/frontend/src/scenes/session-recordings/player/modal/SessionPlayerModal.tsx @@ -48,6 +48,8 @@ export function SessionPlayerModal(): JSX.Element | null { width={1600} fullScreen={isFullScreen} closable={!isFullScreen} + zIndex="1061" + hideCloseButton={true} >
{activeSessionRecording ? ( diff --git a/frontend/src/scenes/session-recordings/player/share/PlayerShare.tsx b/frontend/src/scenes/session-recordings/player/share/PlayerShare.tsx index 98b70822c4cc1..90de41dc35db4 100644 --- a/frontend/src/scenes/session-recordings/player/share/PlayerShare.tsx +++ b/frontend/src/scenes/session-recordings/player/share/PlayerShare.tsx @@ -76,5 +76,6 @@ export function openPlayerShareDialog({ seconds, id }: PlayerShareLogicProps): v children: 'Close', type: 'secondary', }, + zIndex: '1062', }) } diff --git a/frontend/src/scenes/settings/SettingsMap.tsx b/frontend/src/scenes/settings/SettingsMap.tsx index 5611178c4dbd0..5effa45c14085 100644 --- a/frontend/src/scenes/settings/SettingsMap.tsx +++ b/frontend/src/scenes/settings/SettingsMap.tsx @@ -108,7 +108,7 @@ export const SETTINGS_MAP: SettingSection[] = [ id: 'exception-autocapture', title: 'Exception autocapture', component: , - flag: 'EXCEPTION_AUTOCAPTURE', + flag: 'ERROR_TRACKING', }, { id: 'web-vitals-autocapture', diff --git a/frontend/src/scenes/surveys/SurveyView.tsx b/frontend/src/scenes/surveys/SurveyView.tsx index d81d23a1688bb..5b69762e9d487 100644 --- a/frontend/src/scenes/surveys/SurveyView.tsx +++ b/frontend/src/scenes/surveys/SurveyView.tsx @@ -411,12 +411,6 @@ export function SurveyView({ id }: { id: string }): JSX.Element { type: 'events', order: 0, properties: [ - { - key: '$survey_response', - type: PropertyFilterType.Event, - value: 'is_set', - operator: PropertyOperator.IsSet, - }, { key: '$survey_id', type: PropertyFilterType.Event, diff --git a/frontend/src/scenes/surveys/Surveys.tsx b/frontend/src/scenes/surveys/Surveys.tsx index 0e4091361f91d..de73aff73789d 100644 --- a/frontend/src/scenes/surveys/Surveys.tsx +++ b/frontend/src/scenes/surveys/Surveys.tsx @@ -32,7 +32,7 @@ import { SceneExport } from 'scenes/sceneTypes' import { urls } from 'scenes/urls' import { userLogic } from 'scenes/userLogic' -import { ActivityScope, ProductKey, ProgressStatus, PropertyFilterType, PropertyOperator, Survey } from '~/types' +import { ActivityScope, ProductKey, ProgressStatus, Survey } from '~/types' import { SurveyQuestionLabel } from './constants' import { openSurveysSettingsDialog } from './SurveySettings' @@ -127,14 +127,6 @@ export function Surveys(): JSX.Element { id: 'survey sent', type: 'events', order: 0, - properties: [ - { - key: '$survey_response', - type: PropertyFilterType.Event, - value: 'is_set', - operator: PropertyOperator.IsSet, - }, - ], }, ], }} diff --git a/frontend/src/scenes/teamActivityDescriber.tsx b/frontend/src/scenes/teamActivityDescriber.tsx index 45c875676b724..ed3bb43d043b8 100644 --- a/frontend/src/scenes/teamActivityDescriber.tsx +++ b/frontend/src/scenes/teamActivityDescriber.tsx @@ -164,7 +164,7 @@ const teamActionsMapping: Record< return { description: [<>set allowed web vitals autocapture metrics to {metricsList}] } }, autocapture_opt_out(change: ActivityChange | undefined): ChangeMapping | null { - return { description: [<>{change?.after ? 'opted in to' : 'opted out of'} autocapture] } + return { description: [<>{change?.after ? 'opted out of' : 'opted in to'} autocapture] } }, heatmaps_opt_in(change: ActivityChange | undefined): ChangeMapping | null { return { description: [<>{change?.after ? 'enabled' : 'disabled'} heatmaps] } diff --git a/frontend/src/scenes/trends/mathsLogic.tsx b/frontend/src/scenes/trends/mathsLogic.tsx index 1e5c93663bcd9..04756fd135dd9 100644 --- a/frontend/src/scenes/trends/mathsLogic.tsx +++ b/frontend/src/scenes/trends/mathsLogic.tsx @@ -4,7 +4,7 @@ import { groupsAccessLogic } from 'lib/introductions/groupsAccessLogic' import { featureFlagLogic } from 'lib/logic/featureFlagLogic' import { groupsModel } from '~/models/groupsModel' -import { BaseMathType, CountPerActorMathType, HogQLMathType, PropertyMathType } from '~/types' +import { BaseMathType, CountPerActorMathType, FunnelMathType, HogQLMathType, PropertyMathType } from '~/types' import type { mathsLogicType } from './mathsLogicType' @@ -25,6 +25,50 @@ export interface MathDefinition { category: MathCategory } +export const FUNNEL_MATH_DEFINITIONS: Record = { + [FunnelMathType.AnyMatch]: { + name: 'Any events match', + shortName: 'any event', + description: <>Any event of this type that matches the filter will count towards the funnel, + category: MathCategory.EventCount, + }, + [FunnelMathType.FirstTimeForUser]: { + name: 'First event for user', + shortName: 'first event', + description: ( + <> + Only the first time the user performed this event will count towards the funnel, and only if it matches + the event filters. +
+
+ + Example: If the we are looking for pageview events to posthog.com/about, but the user's first + pageview was on posthog.com, it will not match, even if they went to posthog.com/about afterwards. + + + ), + category: MathCategory.EventCount, + }, + [FunnelMathType.FirstTimeForUserWithFilters]: { + name: 'First matching event for user', + shortName: 'first matching event', + description: ( + <> + The first time the user performed this event that matches the event filters will count towards the + funnel. +
+
+ + Example: If the we are looking for pageview events to posthog.com/about, and the user's first + pageview was on posthog.com but then they navigated to posthog.com/about, it will match the pageview + event from posthog.com/about + + + ), + category: MathCategory.EventCount, + }, +} + export const BASE_MATH_DEFINITIONS: Record = { [BaseMathType.TotalCount]: { name: 'Total count', @@ -320,6 +364,15 @@ export const mathsLogic = kea([ return filterMathTypesUnderFeatureFlags(allMathDefinitions, featureFlags) }, ], + funnelMathDefinitions: [ + (s) => [s.featureFlags], + (featureFlags): Record => { + const funnelMathDefinitions: Record = { + ...FUNNEL_MATH_DEFINITIONS, + } + return filterMathTypesUnderFeatureFlags(funnelMathDefinitions, featureFlags) + }, + ], // Static means the options do not have nested selectors (like math function) staticMathDefinitions: [ (s) => [s.groupsMathDefinitions, s.needsUpgradeForGroups, s.featureFlags], diff --git a/frontend/src/scenes/urls.ts b/frontend/src/scenes/urls.ts index a6def0d63f5ff..13e83e799eac7 100644 --- a/frontend/src/scenes/urls.ts +++ b/frontend/src/scenes/urls.ts @@ -3,7 +3,7 @@ import { AlertType } from 'lib/components/Alerts/types' import { getCurrentTeamId } from 'lib/utils/getAppContext' import { ExportOptions } from '~/exporter/types' -import { DashboardFilter, HogQLFilters, Node } from '~/queries/schema' +import { HogQLFilters, Node } from '~/queries/schema' import { ActionType, ActivityTab, @@ -89,12 +89,8 @@ export const urls = { } ).url, insightEdit: (id: InsightShortId): string => `/insights/${id}/edit`, - insightView: (id: InsightShortId, filtersOverride?: DashboardFilter): string => - `/insights/${id}${ - filtersOverride !== undefined - ? `?filters_override=${encodeURIComponent(JSON.stringify(filtersOverride))}` - : '' - }`, + insightView: (id: InsightShortId, dashboardId?: number): string => + `/insights/${id}${dashboardId !== undefined ? `?dashboard=${dashboardId}` : ''}`, insightSubcriptions: (id: InsightShortId): string => `/insights/${id}/subscriptions`, insightSubcription: (id: InsightShortId, subscriptionId: string): string => `/insights/${id}/subscriptions/${subscriptionId}`, diff --git a/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx b/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx index 0cca3f02fdd43..601add71a85a0 100644 --- a/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx +++ b/frontend/src/scenes/web-analytics/webAnalyticsLogic.tsx @@ -7,13 +7,15 @@ import { FEATURE_FLAGS, RETENTION_FIRST_TIME, STALE_EVENT_SECONDS } from 'lib/co import { dayjs } from 'lib/dayjs' import { Link, PostHogComDocsURL } from 'lib/lemon-ui/Link/Link' import { featureFlagLogic } from 'lib/logic/featureFlagLogic' -import { getDefaultInterval, isNotNil, updateDatesWithInterval } from 'lib/utils' +import { getDefaultInterval, isNotNil, objectsEqual, updateDatesWithInterval } from 'lib/utils' import { errorTrackingQuery } from 'scenes/error-tracking/queries' import { urls } from 'scenes/urls' import { + ActionConversionGoal, ActionsNode, AnyEntityNode, + CustomEventConversionGoal, EventsNode, NodeKind, QuerySchema, @@ -1394,7 +1396,7 @@ export const webAnalyticsLogic = kea([ } }), - urlToAction(({ actions }) => ({ + urlToAction(({ actions, values }) => ({ '/web': ( _, { @@ -1415,36 +1417,46 @@ export const webAnalyticsLogic = kea([ ) => { const parsedFilters = isWebAnalyticsPropertyFilters(filters) ? filters : undefined - if (parsedFilters) { + if (parsedFilters && !objectsEqual(parsedFilters, values.webAnalyticsFilters)) { actions.setWebAnalyticsFilters(parsedFilters) } - if (conversionGoalActionId) { + if ( + conversionGoalActionId && + conversionGoalActionId !== (values.conversionGoal as ActionConversionGoal)?.actionId + ) { actions.setConversionGoal({ actionId: parseInt(conversionGoalActionId, 10) }) - } else if (conversionGoalCustomEventName) { + } else if ( + conversionGoalCustomEventName && + conversionGoalCustomEventName !== (values.conversionGoal as CustomEventConversionGoal)?.customEventName + ) { actions.setConversionGoal({ customEventName: conversionGoalCustomEventName }) } - if (date_from || date_to || interval) { + if ( + (date_from && date_from !== values.dateFilter.dateFrom) || + (date_to && date_to !== values.dateFilter.dateTo) || + (interval && interval !== values.dateFilter.interval) + ) { actions.setDatesAndInterval(date_from, date_to, interval) } - if (device_tab) { + if (device_tab && device_tab !== values._deviceTab) { actions.setDeviceTab(device_tab) } - if (source_tab) { + if (source_tab && source_tab !== values._sourceTab) { actions.setSourceTab(source_tab) } - if (graphs_tab) { + if (graphs_tab && graphs_tab !== values._graphsTab) { actions.setGraphsTab(graphs_tab) } - if (path_tab) { + if (path_tab && path_tab !== values._pathTab) { actions.setPathTab(path_tab) } - if (geography_tab) { + if (geography_tab && geography_tab !== values._geographyTab) { actions.setGeographyTab(geography_tab) } - if (path_cleaning) { + if (path_cleaning && path_cleaning !== values.isPathCleaningEnabled) { actions.setIsPathCleaningEnabled([true, 'true', 1, '1'].includes(path_cleaning)) } - if (filter_test_accounts) { + if (filter_test_accounts && filter_test_accounts !== values.shouldFilterTestAccounts) { actions.setShouldFilterTestAccounts([true, 'true', 1, '1'].includes(filter_test_accounts)) } }, diff --git a/frontend/src/styles/vars.scss b/frontend/src/styles/vars.scss index 115b7da6dac46..9ca16b7964f3d 100644 --- a/frontend/src/styles/vars.scss +++ b/frontend/src/styles/vars.scss @@ -194,9 +194,13 @@ $_lifecycle_dormant: map.get($colors, 'danger'); --z-command-palette: 1875; --z-force-modal-above-popovers: 1850; --z-tooltip: 1070; - --z-definition-popover: 1064; - --z-popover: 1063; - --z-graph-tooltip: 1062; + + // 1066 through 1069 are reserved to be set from code + --z-definition-popover: 1065; + --z-popover: 1064; + --z-graph-tooltip: 1063; + + // 1061 and 1062 are reserved to be set from code --z-modal: 1060; --z-hedgehog-buddy: 1059; --z-annotation-popover: 1049; diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 7357eb4f939e8..33cacd4590382 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -3514,6 +3514,12 @@ export interface InstanceSetting { is_secret: boolean } +export enum FunnelMathType { + AnyMatch = 'total', + FirstTimeForUser = 'first_time_for_user', + FirstTimeForUserWithFilters = 'first_time_for_user_with_filters', +} + export enum BaseMathType { TotalCount = 'total', UniqueUsers = 'dau', @@ -4300,6 +4306,7 @@ export enum SidePanelTab { Discussion = 'discussion', Status = 'status', Exports = 'exports', + ExperimentFeatureFlag = 'experiment-feature-flag', } export interface SourceFieldOauthConfig { diff --git a/latest_migrations.manifest b/latest_migrations.manifest index 7307753ccc3da..ed247df68feb6 100644 --- a/latest_migrations.manifest +++ b/latest_migrations.manifest @@ -5,7 +5,7 @@ contenttypes: 0002_remove_content_type_name ee: 0016_rolemembership_organization_member otp_static: 0002_throttling otp_totp: 0002_auto_20190420_0723 -posthog: 0484_productintent +posthog: 0486_cohort_last_error_at sessions: 0001_initial social_django: 0010_uid_db_index two_factor: 0007_auto_20201201_1019 diff --git a/package.json b/package.json index 49cbf4ef7a91c..910ebd8c5c2e9 100644 --- a/package.json +++ b/package.json @@ -150,7 +150,7 @@ "pmtiles": "^2.11.0", "postcss": "^8.4.31", "postcss-preset-env": "^9.3.0", - "posthog-js": "1.166.0", + "posthog-js": "1.167.0", "posthog-js-lite": "3.0.0", "prettier": "^2.8.8", "prop-types": "^15.7.2", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8b640fe176931..155c3a235f721 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -272,8 +272,8 @@ dependencies: specifier: ^9.3.0 version: 9.3.0(postcss@8.4.31) posthog-js: - specifier: 1.166.0 - version: 1.166.0 + specifier: 1.167.0 + version: 1.167.0 posthog-js-lite: specifier: 3.0.0 version: 3.0.0 @@ -13913,7 +13913,7 @@ packages: hogan.js: 3.0.2 htm: 3.1.1 instantsearch-ui-components: 0.3.0 - preact: 10.24.1 + preact: 10.24.2 qs: 6.9.7 search-insights: 2.13.0 dev: false @@ -17728,11 +17728,11 @@ packages: resolution: {integrity: sha512-dyajjnfzZD1tht4N7p7iwf7nBnR1MjVaVu+MKr+7gBgA39bn28wizCIJZztZPtHy4PY0YwtSGgwfBCuG/hnHgA==} dev: false - /posthog-js@1.166.0: - resolution: {integrity: sha512-om7rhbgP3OzJDJ3wdrp6cuKVWC+7iiFeVv+g8uaZPLI6zvzuaNdq4a3s0ltnfBpROlZrWw4oJknoVwj7I3hVTQ==} + /posthog-js@1.167.0: + resolution: {integrity: sha512-/zXQ6tuJgiF1d4mgg3UsAi/uoyg7UnfFNQtikuALmaE53xFExpcAKbMfHPG/f54QgTvLxSHyGL1kFl/1uspkGg==} dependencies: fflate: 0.4.8 - preact: 10.24.1 + preact: 10.24.2 web-vitals: 4.2.3 dev: false @@ -17740,8 +17740,8 @@ packages: resolution: {integrity: sha512-Q+/tYsFU9r7xoOJ+y/ZTtdVQwTWfzjbiXBDMM/JKUux3+QPP02iUuIoeBQ+Ot6oEDlC+/PGjB/5A3K7KKb7hcw==} dev: false - /preact@10.24.1: - resolution: {integrity: sha512-PnBAwFI3Yjxxcxw75n6VId/5TFxNW/81zexzWD9jn1+eSrOP84NdsS38H5IkF/UH3frqRPT+MvuCoVHjTDTnDw==} + /preact@10.24.2: + resolution: {integrity: sha512-1cSoF0aCC8uaARATfrlz4VCBqE8LwZwRfLgkxJOQwAlQt6ayTmi0D9OF7nXid1POI5SZidFuG9CnlXbDfLqY/Q==} dev: false /prelude-ls@1.2.1: diff --git a/posthog/api/cohort.py b/posthog/api/cohort.py index f6e92d257b40a..af5eb50e8f4b7 100644 --- a/posthog/api/cohort.py +++ b/posthog/api/cohort.py @@ -610,13 +610,15 @@ def insert_actors_into_cohort_by_query(cohort: Cohort, query: str, params: dict[ cohort.is_calculating = False cohort.last_calculation = timezone.now() cohort.errors_calculating = 0 - cohort.save(update_fields=["errors_calculating", "last_calculation", "is_calculating"]) + cohort.last_error_at = None + cohort.save(update_fields=["errors_calculating", "last_calculation", "is_calculating", "last_error_at"]) except Exception as err: if settings.DEBUG: raise cohort.is_calculating = False cohort.errors_calculating = F("errors_calculating") + 1 - cohort.save(update_fields=["errors_calculating", "is_calculating"]) + cohort.last_error_at = timezone.now() + cohort.save(update_fields=["errors_calculating", "is_calculating", "last_error_at"]) capture_exception(err) diff --git a/posthog/api/insight.py b/posthog/api/insight.py index 1efbc0a74a538..f27a1f41e559f 100644 --- a/posthog/api/insight.py +++ b/posthog/api/insight.py @@ -647,7 +647,7 @@ def insight_result(self, insight: Insight) -> InsightResult: insight, dashboard=dashboard, execution_mode=execution_mode, - user=self.context["request"].user, + user=None if self.context["request"].user.is_anonymous else self.context["request"].user, filters_override=filters_override, ) except ExposedHogQLError as e: diff --git a/posthog/api/test/__snapshots__/test_decide.ambr b/posthog/api/test/__snapshots__/test_decide.ambr index 55d6d13064021..7d14f67211293 100644 --- a/posthog/api/test/__snapshots__/test_decide.ambr +++ b/posthog/api/test/__snapshots__/test_decide.ambr @@ -842,6 +842,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" @@ -866,6 +867,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" @@ -1092,6 +1094,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" @@ -1128,6 +1131,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" diff --git a/posthog/api/test/__snapshots__/test_feature_flag.ambr b/posthog/api/test/__snapshots__/test_feature_flag.ambr index 57d56d8233aba..7be6faab8299e 100644 --- a/posthog/api/test/__snapshots__/test_feature_flag.ambr +++ b/posthog/api/test/__snapshots__/test_feature_flag.ambr @@ -375,6 +375,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" @@ -621,6 +622,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" @@ -727,6 +729,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" @@ -949,6 +952,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" @@ -1257,6 +1261,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" @@ -1324,6 +1329,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" @@ -1969,6 +1975,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" diff --git a/posthog/api/test/batch_exports/operations.py b/posthog/api/test/batch_exports/operations.py index 20f7d2761e2bf..625a22f5c4304 100644 --- a/posthog/api/test/batch_exports/operations.py +++ b/posthog/api/test/batch_exports/operations.py @@ -1,5 +1,6 @@ from django.test.client import Client as TestClient from rest_framework import status + from posthog.models.utils import UUIDT @@ -120,3 +121,13 @@ def get_batch_export_log_entries(client: TestClient, team_id: int, batch_export_ def get_batch_export_run_log_entries(client: TestClient, team_id: int, batch_export_id: str, run_id, **extra): return client.get(f"/api/projects/{team_id}/batch_exports/{batch_export_id}/runs/{run_id}/logs", extra) + + +def cancel_batch_export_run(client: TestClient, team_id: int, batch_export_id: str, run_id): + return client.post(f"/api/projects/{team_id}/batch_exports/{batch_export_id}/runs/{run_id}/cancel") + + +def cancel_batch_export_run_ok(client: TestClient, team_id: int, batch_export_id: str, run_id): + response = client.post(f"/api/projects/{team_id}/batch_exports/{batch_export_id}/runs/{run_id}/cancel") + assert response.status_code == status.HTTP_200_OK, response.json() + return response.json() diff --git a/posthog/api/test/batch_exports/test_backfill.py b/posthog/api/test/batch_exports/test_backfill.py index 8a760e5e7c0cd..e8dd4a29e81db 100644 --- a/posthog/api/test/batch_exports/test_backfill.py +++ b/posthog/api/test/batch_exports/test_backfill.py @@ -312,3 +312,56 @@ def test_backfill_is_partitioned_by_team_id(client: HttpClient): ) assert response.status_code == status.HTTP_404_NOT_FOUND, response.json() + + +def test_batch_export_backfill_created_in_timezone(client: HttpClient): + """Test creating a BatchExportBackfill sets the right ID in UTC timezone. + + PostgreSQL stores datetime values in their UTC representation, converting the input + if it's in a different timezone. For this reason, we need backfills to have a workflow + ID in UTC representation, so that we can later re-construct this ID from the data stored + in PostgreSQL. + + Otherwise, we would need to store a timezone field in PostgreSQL too. We may want + to do that later, but this test case is still valuable to ensure we are pulling and + using the timezone stored in PostgreSQL correctly. + """ + temporal = sync_connect() + + destination_data = { + "type": "S3", + "config": { + "bucket_name": "my-production-s3-bucket", + "region": "us-east-1", + "prefix": "posthog-events/", + "aws_access_key_id": "abc123", + "aws_secret_access_key": "secret", + }, + } + batch_export_data = { + "name": "my-production-s3-bucket-destination", + "destination": destination_data, + "interval": "hour", + } + + organization = create_organization("Test Org") + team = create_team(organization, timezone="US/Eastern") + user = create_user("test@user.com", "Test User", organization) + client.force_login(user) + + with start_test_worker(temporal): + batch_export = create_batch_export_ok(client, team.pk, batch_export_data) + batch_export_id = batch_export["id"] + + response = backfill_batch_export( + client, + team.pk, + batch_export_id, + "2021-01-01T00:00:00-05:00", + "2021-10-01T00:00:00-04:00", + ) + + data = response.json() + + assert response.status_code == status.HTTP_200_OK, data + assert data["backfill_id"] == f"{batch_export_id}-Backfill-2021-01-01T05:00:00+00:00-2021-10-01T04:00:00+00:00" diff --git a/posthog/api/test/batch_exports/test_runs.py b/posthog/api/test/batch_exports/test_runs.py index feac9b5c63e18..f8ec68a623641 100644 --- a/posthog/api/test/batch_exports/test_runs.py +++ b/posthog/api/test/batch_exports/test_runs.py @@ -1,12 +1,19 @@ +import asyncio + import pytest +import temporalio.client +from asgiref.sync import async_to_sync from django.test.client import Client as HttpClient from rest_framework import status from posthog.api.test.batch_exports.conftest import start_test_worker from posthog.api.test.batch_exports.operations import ( + backfill_batch_export_ok, + cancel_batch_export_run_ok, create_batch_export_ok, get_batch_export, get_batch_export_runs, + get_batch_export_runs_ok, ) from posthog.api.test.test_organization import create_organization from posthog.api.test.test_team import create_team @@ -143,3 +150,82 @@ def test_batch_exports_are_partitioned_by_team(client: HttpClient): response = get_batch_export(client, team.pk, batch_export["id"]) assert response.status_code == status.HTTP_404_NOT_FOUND, response.json() + + +# TODO - this was in test_delete.py too so maybe extract it out into operations.py? +@async_to_sync +async def wait_for_workflow_executions( + temporal: temporalio.client.Client, query: str, timeout: int = 30, sleep: int = 1 +): + """Wait for Workflow Executions matching query.""" + workflows = [workflow async for workflow in temporal.list_workflows(query=query)] + + total = 0 + while not workflows: + total += sleep + + if total > timeout: + raise TimeoutError(f"No backfill Workflow Executions after {timeout} seconds") + + await asyncio.sleep(sleep) + workflows = [workflow async for workflow in temporal.list_workflows(query=query)] + + return workflows + + +@pytest.mark.django_db(transaction=True) +def test_cancelling_a_batch_export_run(client: HttpClient): + """Test cancelling a BatchExportRun.""" + temporal = sync_connect() + + destination_data = { + "type": "S3", + "config": { + "bucket_name": "my-production-s3-bucket", + "region": "us-east-1", + "prefix": "posthog-events/", + "aws_access_key_id": "abc123", + "aws_secret_access_key": "secret", + }, + } + batch_export_data = { + "name": "my-production-s3-bucket-destination", + "destination": destination_data, + "interval": "hour", + } + + organization = create_organization("Test Org") + team = create_team(organization) + user = create_user("test@user.com", "Test User", organization) + client.force_login(user) + + with start_test_worker(temporal): + batch_export = create_batch_export_ok( + client, + team.pk, + batch_export_data, + ) + batch_export_id = batch_export["id"] + + start_at = "2023-10-23T00:00:00+00:00" + end_at = "2023-10-24T00:00:00+00:00" + backfill_batch_export_ok(client, team.pk, batch_export_id, start_at, end_at) + + # In order for a run to be cancelable we need a running workflow execution + _ = wait_for_workflow_executions(temporal, query=f'TemporalScheduledById="{batch_export_id}"') + + data = get_batch_export_runs_ok(client, team.pk, batch_export_id) + assert len(data["results"]) == 1 + run = data["results"][0] + assert run["status"] == "Running" + + data = cancel_batch_export_run_ok(client, team.pk, batch_export_id, run["id"]) + assert data["cancelled"] is True + + data = get_batch_export_runs_ok(client, team.pk, batch_export_id) + assert len(data["results"]) == 1 + run = data["results"][0] + assert run["status"] == "Cancelled" + + +# TODO - add a test to ensure we can't cancel a completed run? diff --git a/posthog/batch_exports/http.py b/posthog/batch_exports/http.py index bd56a092771e3..8698e4c450a29 100644 --- a/posthog/batch_exports/http.py +++ b/posthog/batch_exports/http.py @@ -6,7 +6,6 @@ from django.db import transaction from django.utils.timezone import now from rest_framework import filters, request, response, serializers, viewsets -from posthog.api.utils import action from rest_framework.exceptions import ( NotAuthenticated, NotFound, @@ -17,6 +16,7 @@ from posthog.api.log_entries import LogEntryMixin from posthog.api.routing import TeamAndOrgViewSetMixin +from posthog.api.utils import action from posthog.batch_exports.models import BATCH_EXPORT_INTERVALS from posthog.batch_exports.service import ( BatchExportIdError, @@ -25,6 +25,7 @@ BatchExportServiceRPCError, BatchExportWithNoEndNotAllowedError, backfill_export, + cancel_running_batch_export_run, disable_and_delete_export, pause_batch_export, sync_batch_export, @@ -148,6 +149,18 @@ def retry(self, *args, **kwargs) -> response.Response: return response.Response({"backfill_id": backfill_id}) + @action(methods=["POST"], detail=True, required_scopes=["batch_export:write"]) + def cancel(self, *args, **kwargs) -> response.Response: + """Cancel a batch export run.""" + + batch_export_run: BatchExportRun = self.get_object() + + temporal = sync_connect() + # TODO: check status of run beforehand + cancel_running_batch_export_run(temporal, batch_export_run) + + return response.Response({"cancelled": True}) + class BatchExportDestinationSerializer(serializers.ModelSerializer): """Serializer for an BatchExportDestination model.""" diff --git a/posthog/batch_exports/models.py b/posthog/batch_exports/models.py index e319d73db30ab..da3a4576b6682 100644 --- a/posthog/batch_exports/models.py +++ b/posthog/batch_exports/models.py @@ -107,6 +107,11 @@ class Status(models.TextChoices): null=True, help_text="The total count of records that should be exported in this BatchExportRun." ) + @property + def workflow_id(self) -> str: + """Return the Workflow id that corresponds to this BatchExportRun model.""" + return f"{self.batch_export.id}-{self.data_interval_end:%Y-%m-%dT%H:%M:%S}Z" + def fetch_batch_export_run_count( *, diff --git a/posthog/batch_exports/service.py b/posthog/batch_exports/service.py index 49cccbac142e2..015dd01c6b4fa 100644 --- a/posthog/batch_exports/service.py +++ b/posthog/batch_exports/service.py @@ -357,7 +357,14 @@ def disable_and_delete_export(instance: BatchExport): instance.deleted = True for backfill in running_backfills_for_batch_export(instance.id): - async_to_sync(cancel_running_batch_export_backfill)(temporal, backfill) + try: + async_to_sync(cancel_running_batch_export_backfill)(temporal, backfill) + except Exception: + logger.exception( + "Failed to delete backfill %s for batch export %s, but will continue on with delete", + backfill.id, + instance.id, + ) try: batch_export_delete_schedule(temporal, str(instance.pk)) @@ -402,6 +409,16 @@ async def cancel_running_batch_export_backfill(temporal: Client, batch_export_ba await batch_export_backfill.asave() +def cancel_running_batch_export_run(temporal: Client, batch_export_run: BatchExportRun) -> None: + """Cancel a running BatchExportRun.""" + + handle = temporal.get_workflow_handle(workflow_id=batch_export_run.workflow_id) + async_to_sync(handle.cancel)() + + batch_export_run.status = BatchExportRun.Status.CANCELLED + batch_export_run.save() + + @dataclass class BackfillBatchExportInputs: """Inputs for the BackfillBatchExport Workflow.""" @@ -449,14 +466,22 @@ def backfill_export( start_at=start_at.isoformat(), end_at=end_at.isoformat() if end_at else None, ) - workflow_id = start_backfill_batch_export_workflow(temporal, inputs=inputs) + start_at_utc_str = start_at.astimezone(tz=dt.UTC).isoformat() + # TODO: Should we use another signal besides "None"? i.e. "Inf" or "END". + # Keeping it like this for now for backwards compatibility. + end_at_utc_str = end_at.astimezone(tz=dt.UTC).isoformat() if end_at else "None" + + workflow_id = f"{inputs.batch_export_id}-Backfill-{start_at_utc_str}-{end_at_utc_str}" + + workflow_id = start_backfill_batch_export_workflow(temporal, workflow_id, inputs=inputs) return workflow_id @async_to_sync -async def start_backfill_batch_export_workflow(temporal: Client, inputs: BackfillBatchExportInputs) -> str: +async def start_backfill_batch_export_workflow( + temporal: Client, workflow_id: str, inputs: BackfillBatchExportInputs +) -> str: """Async call to start a BackfillBatchExportWorkflow.""" - workflow_id = f"{inputs.batch_export_id}-Backfill-{inputs.start_at}-{inputs.end_at}" await temporal.start_workflow( "backfill-batch-export", inputs, diff --git a/posthog/batch_exports/sql.py b/posthog/batch_exports/sql.py index b61bd1d448006..e7d4b95c9fe6a 100644 --- a/posthog/batch_exports/sql.py +++ b/posthog/batch_exports/sql.py @@ -130,7 +130,7 @@ event AS event, distinct_id AS distinct_id, toString(uuid) AS uuid, - COALESCE(inserted_at, _timestamp) AS _inserted_at, + timestamp AS _inserted_at, created_at AS created_at, elements_chain AS elements_chain, toString(person_id) AS person_id, diff --git a/posthog/caching/utils.py b/posthog/caching/utils.py index 1f74694bbe7ba..28f142bf78d65 100644 --- a/posthog/caching/utils.py +++ b/posthog/caching/utils.py @@ -1,10 +1,9 @@ -from datetime import datetime, timedelta, UTC +from datetime import UTC, datetime, timedelta from enum import Enum from typing import Any, Optional, Union -from dateutil.parser import parser, isoparse - import posthoganalytics +from dateutil.parser import isoparse, parser from posthog.client import sync_execute from posthog.cloud_utils import is_cloud @@ -125,9 +124,10 @@ class ThresholdMode(Enum): LEGACY = "legacy" DEFAULT = "default" LAZY = "lazy" + AI = "ai" -staleness_threshold_map = { +staleness_threshold_map: dict[ThresholdMode, dict[Optional[str], timedelta]] = { ThresholdMode.DEFAULT: { None: timedelta(minutes=1), "minute": timedelta(seconds=15), @@ -144,6 +144,9 @@ class ThresholdMode(Enum): "week": timedelta(days=1), "month": timedelta(days=2), }, + ThresholdMode.AI: { + None: timedelta(hours=1), + }, } diff --git a/posthog/clickhouse/migrations/0080_recreate_events_backfill.py b/posthog/clickhouse/migrations/0080_recreate_events_backfill.py new file mode 100644 index 0000000000000..f18706e1c5142 --- /dev/null +++ b/posthog/clickhouse/migrations/0080_recreate_events_backfill.py @@ -0,0 +1,11 @@ +from posthog.batch_exports.sql import ( + CREATE_EVENTS_BATCH_EXPORT_VIEW_BACKFILL, +) +from posthog.clickhouse.client.migration_tools import run_sql_with_exceptions + +operations = map( + run_sql_with_exceptions, + [ + CREATE_EVENTS_BATCH_EXPORT_VIEW_BACKFILL, + ], +) diff --git a/posthog/hogql/database/database.py b/posthog/hogql/database/database.py index cd34885f24e3a..e1e3fd26f8203 100644 --- a/posthog/hogql/database/database.py +++ b/posthog/hogql/database/database.py @@ -167,7 +167,7 @@ def get_table(self, table_name: str) -> Table: raise QueryError(f'Unknown table "{table_name}".') def get_all_tables(self) -> list[str]: - return self._table_names + self._warehouse_table_names + return self._table_names + self._warehouse_table_names + self._view_table_names def get_posthog_tables(self) -> list[str]: return self._table_names diff --git a/posthog/hogql/functions/mapping.py b/posthog/hogql/functions/mapping.py index dfe50a5b67a2b..08bee305b933e 100644 --- a/posthog/hogql/functions/mapping.py +++ b/posthog/hogql/functions/mapping.py @@ -2,6 +2,7 @@ from itertools import chain from typing import Optional + from posthog.cloud_utils import is_cloud, is_ci from posthog.hogql import ast from posthog.hogql.ast import ( @@ -80,16 +81,13 @@ class HogQLFunctionMeta: def compare_types(arg_types: list[ConstantType], sig_arg_types: tuple[ConstantType, ...]): - _sig_arg_types = list(sig_arg_types) if len(arg_types) != len(sig_arg_types): return False - for index, arg_type in enumerate(arg_types): - _sig_arg_type = _sig_arg_types[index] - if not isinstance(arg_type, _sig_arg_type.__class__): - return False - - return True + return all( + isinstance(sig_arg_type, UnknownType) or isinstance(arg_type, sig_arg_type.__class__) + for arg_type, sig_arg_type in zip(arg_types, sig_arg_types) + ) HOGQL_COMPARISON_MAPPING: dict[str, ast.CompareOperationOp] = { @@ -342,6 +340,7 @@ def compare_types(arg_types: list[ConstantType], sig_arg_types: tuple[ConstantTy "arraySplit": HogQLFunctionMeta("arraySplit", 2, None), "arrayReverseFill": HogQLFunctionMeta("arrayReverseFill", 2, None), "arrayReverseSplit": HogQLFunctionMeta("arrayReverseSplit", 2, None), + "arrayRotateRight": HogQLFunctionMeta("arrayRotateRight", 2, 2), "arrayExists": HogQLFunctionMeta("arrayExists", 1, None), "arrayAll": HogQLFunctionMeta("arrayAll", 1, None), "arrayFirst": HogQLFunctionMeta("arrayFirst", 2, None), @@ -447,15 +446,59 @@ def compare_types(arg_types: list[ConstantType], sig_arg_types: tuple[ConstantTy "toStartOfYear": HogQLFunctionMeta("toStartOfYear", 1, 1), "toStartOfISOYear": HogQLFunctionMeta("toStartOfISOYear", 1, 1), "toStartOfQuarter": HogQLFunctionMeta("toStartOfQuarter", 1, 1), - "toStartOfMonth": HogQLFunctionMeta("toStartOfMonth", 1, 1), + "toStartOfMonth": HogQLFunctionMeta( + "toStartOfMonth", + 1, + 1, + signatures=[ + ((UnknownType(),), DateType()), + ], + ), "toLastDayOfMonth": HogQLFunctionMeta("toLastDayOfMonth", 1, 1), "toMonday": HogQLFunctionMeta("toMonday", 1, 1), - "toStartOfWeek": HogQLFunctionMeta("toStartOfWeek", 1, 2), - "toStartOfDay": HogQLFunctionMeta("toStartOfDay", 1, 2), + "toStartOfWeek": HogQLFunctionMeta( + "toStartOfWeek", + 1, + 2, + signatures=[ + ((UnknownType(),), DateType()), + ((UnknownType(), UnknownType()), DateType()), + ], + ), + "toStartOfDay": HogQLFunctionMeta( + "toStartOfDay", + 1, + 2, + signatures=[ + ((UnknownType(),), DateTimeType()), + ((UnknownType(), UnknownType()), DateTimeType()), + ], + ), "toLastDayOfWeek": HogQLFunctionMeta("toLastDayOfWeek", 1, 2), - "toStartOfHour": HogQLFunctionMeta("toStartOfHour", 1, 1), - "toStartOfMinute": HogQLFunctionMeta("toStartOfMinute", 1, 1), - "toStartOfSecond": HogQLFunctionMeta("toStartOfSecond", 1, 1), + "toStartOfHour": HogQLFunctionMeta( + "toStartOfHour", + 1, + 1, + signatures=[ + ((UnknownType(),), DateTimeType()), + ], + ), + "toStartOfMinute": HogQLFunctionMeta( + "toStartOfMinute", + 1, + 1, + signatures=[ + ((UnknownType(),), DateTimeType()), + ], + ), + "toStartOfSecond": HogQLFunctionMeta( + "toStartOfSecond", + 1, + 1, + signatures=[ + ((UnknownType(),), DateTimeType()), + ], + ), "toStartOfFiveMinutes": HogQLFunctionMeta("toStartOfFiveMinutes", 1, 1), "toStartOfTenMinutes": HogQLFunctionMeta("toStartOfTenMinutes", 1, 1), "toStartOfFifteenMinutes": HogQLFunctionMeta("toStartOfFifteenMinutes", 1, 1), @@ -471,7 +514,17 @@ def compare_types(arg_types: list[ConstantType], sig_arg_types: tuple[ConstantTy "dateSub": HogQLFunctionMeta("dateSub", 3, 3), "timeStampAdd": HogQLFunctionMeta("timeStampAdd", 2, 2), "timeStampSub": HogQLFunctionMeta("timeStampSub", 2, 2), - "now": HogQLFunctionMeta("now64", 0, 1, tz_aware=True, case_sensitive=False), + "now": HogQLFunctionMeta( + "now64", + 0, + 1, + tz_aware=True, + case_sensitive=False, + signatures=[ + ((), DateTimeType()), + ((UnknownType(),), DateTimeType()), + ], + ), "nowInBlock": HogQLFunctionMeta("nowInBlock", 1, 1), "rowNumberInBlock": HogQLFunctionMeta("rowNumberInBlock", 0, 0), "rowNumberInAllBlocks": HogQLFunctionMeta("rowNumberInAllBlocks", 0, 0), diff --git a/posthog/hogql/test/test_autocomplete.py b/posthog/hogql/test/test_autocomplete.py index 936944057289b..04f62bb0bdabc 100644 --- a/posthog/hogql/test/test_autocomplete.py +++ b/posthog/hogql/test/test_autocomplete.py @@ -8,6 +8,9 @@ from posthog.models.property_definition import PropertyDefinition from posthog.schema import HogQLAutocomplete, HogQLAutocompleteResponse, HogLanguage, HogQLQuery, Kind from posthog.test.base import APIBaseTest, ClickhouseTestMixin +from posthog.warehouse.models.credential import DataWarehouseCredential +from posthog.warehouse.models.datawarehouse_saved_query import DataWarehouseSavedQuery +from posthog.warehouse.models.table import DataWarehouseTable class TestAutocomplete(ClickhouseTestMixin, APIBaseTest): @@ -412,3 +415,28 @@ def test_autocomplete_variables_prefix(self): assert len(results.suggestions) == 1 assert results.suggestions[0].label == "variable_1" + + def test_autocomplete_warehouse_table(self): + credentials = DataWarehouseCredential.objects.create(team=self.team, access_key="key", access_secret="secret") + DataWarehouseTable.objects.create( + team=self.team, + name="some_table", + format="CSV", + url_pattern="http://localhost/file.csv", + credential=credentials, + ) + query = "select * from " + results = self._select(query=query, start=14, end=14) + + assert "some_table" in [x.label for x in results.suggestions] + + def test_autocomplete_warehouse_view(self): + DataWarehouseSavedQuery.objects.create( + team=self.team, + name="some_view", + query={"query": "select * from events"}, + ) + query = "select * from " + results = self._select(query=query, start=14, end=14) + + assert "some_view" in [x.label for x in results.suggestions] diff --git a/posthog/hogql/test/test_mapping.py b/posthog/hogql/test/test_mapping.py index 6e074687d22cf..8dc805dace346 100644 --- a/posthog/hogql/test/test_mapping.py +++ b/posthog/hogql/test/test_mapping.py @@ -1,4 +1,8 @@ -from posthog.hogql.ast import FloatType, IntegerType +from posthog.hogql.ast import FloatType, IntegerType, DateType +from posthog.hogql.base import UnknownType +from posthog.hogql.context import HogQLContext +from posthog.hogql.parser import parse_expr +from posthog.hogql.printer import print_ast from posthog.test.base import BaseTest from typing import Optional from posthog.hogql.functions.mapping import ( @@ -7,6 +11,7 @@ find_hogql_aggregation, find_hogql_posthog_function, HogQLFunctionMeta, + HOGQL_CLICKHOUSE_FUNCTIONS, ) @@ -60,3 +65,26 @@ def test_compare_types_mismatch_lengths(self): def test_compare_types_mismatch_differing_order(self): res = compare_types([IntegerType(), FloatType()], (FloatType(), IntegerType())) assert res is False + + def test_unknown_type_mapping(self): + HOGQL_CLICKHOUSE_FUNCTIONS["overloadedFunction"] = HogQLFunctionMeta( + "overloadFailure", + 1, + 1, + overloads=[((DateType,), "overloadSuccess")], + ) + + HOGQL_CLICKHOUSE_FUNCTIONS["dateEmittingFunction"] = HogQLFunctionMeta( + "dateEmittingFunction", + 1, + 1, + signatures=[ + ((UnknownType(),), DateType()), + ], + ) + ast = print_ast( + parse_expr("overloadedFunction(dateEmittingFunction('123123'))"), + HogQLContext(self.team.pk, enable_select_queries=True), + "clickhouse", + ) + assert "overloadSuccess" in ast diff --git a/posthog/hogql_queries/ai/__init__.py b/posthog/hogql_queries/ai/__init__.py new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/posthog/hogql_queries/ai/event_taxonomy_query_runner.py b/posthog/hogql_queries/ai/event_taxonomy_query_runner.py new file mode 100644 index 0000000000000..23530e1faf259 --- /dev/null +++ b/posthog/hogql_queries/ai/event_taxonomy_query_runner.py @@ -0,0 +1,145 @@ +from datetime import datetime +from typing import Optional, cast + +from posthog.caching.utils import ThresholdMode, is_stale +from posthog.hogql import ast +from posthog.hogql.parser import parse_expr, parse_select +from posthog.hogql.printer import to_printed_hogql +from posthog.hogql.query import execute_hogql_query +from posthog.hogql_queries.query_runner import QueryRunner +from posthog.schema import ( + CachedEventTaxonomyQueryResponse, + EventTaxonomyItem, + EventTaxonomyQuery, + EventTaxonomyQueryResponse, +) + + +class EventTaxonomyQueryRunner(QueryRunner): + query: EventTaxonomyQuery + response: EventTaxonomyQueryResponse + cached_response: CachedEventTaxonomyQueryResponse + + def calculate(self): + query = self.to_query() + hogql = to_printed_hogql(query, self.team) + + response = execute_hogql_query( + query_type="EventTaxonomyQuery", + query=query, + team=self.team, + timings=self.timings, + modifiers=self.modifiers, + limit_context=self.limit_context, + ) + + results: list[EventTaxonomyItem] = [] + for prop, sample_values, sample_count in response.results: + results.append( + EventTaxonomyItem( + property=prop, + sample_values=sample_values, + sample_count=sample_count, + ) + ) + + return EventTaxonomyQueryResponse( + results=results, + timings=response.timings, + hogql=hogql, + modifiers=self.modifiers, + ) + + def to_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: + query = parse_select( + """ + SELECT + key, + -- Pick five latest distinct sample values. + arraySlice(arrayDistinct(groupArray(value)), 1, 5) AS values, + count(distinct value) AS total_count + FROM {from_query} + ARRAY JOIN kv.1 AS key, kv.2 AS value + WHERE {filter} + GROUP BY key + ORDER BY total_count DESC + """, + placeholders={"from_query": self._get_subquery(), "filter": self._get_omit_filter()}, + ) + + return query + + def _is_stale(self, last_refresh: Optional[datetime], lazy: bool = False) -> bool: + """ + Despite the lazy mode, it caches for an hour by default. We don't want frequent updates here. + """ + return is_stale(self.team, date_to=None, interval=None, last_refresh=last_refresh, mode=ThresholdMode.AI) + + def cache_target_age(self, last_refresh: Optional[datetime], lazy: bool = False) -> Optional[datetime]: + return None + + def _get_omit_filter(self): + """ + Ignore properties that are not useful for AI. + """ + omit_list = [ + # events + r"\$set", + r"\$time", + r"\$set_once", + r"\$sent_at", + # privacy-related + r"\$ip", + # flatten-properties-plugin + "__", + # other metadata + "phjs", + "survey_dismissed", + "survey_responded", + "partial_filter_chosen", + "changed_action", + "window-id", + "changed_event", + "partial_filter", + ] + regex_conditions = "|".join(omit_list) + + return ast.Not( + expr=ast.Call( + name="match", + args=[ + ast.Field(chain=["key"]), + ast.Constant(value=f"({regex_conditions})"), + ], + ) + ) + + def _get_subquery_filter(self) -> ast.Expr: + date_filter = parse_expr("timestamp >= now() - INTERVAL 30 DAY") + filter_expr = ast.And( + exprs=[ + date_filter, + ast.CompareOperation( + left=ast.Field(chain=["event"]), + right=ast.Constant(value=self.query.event), + op=ast.CompareOperationOp.Eq, + ), + ] + ) + return filter_expr + + def _get_subquery(self) -> ast.SelectQuery: + query = parse_select( + """ + SELECT + JSONExtractKeysAndValues(properties, 'String') as kv + FROM + events + WHERE {filter} + ORDER BY timestamp desc + LIMIT 100 + """, + placeholders={"filter": self._get_subquery_filter()}, + ) + + return cast(ast.SelectQuery, query) diff --git a/posthog/hogql_queries/ai/team_taxonomy_query_runner.py b/posthog/hogql_queries/ai/team_taxonomy_query_runner.py new file mode 100644 index 0000000000000..efa25ee17f68b --- /dev/null +++ b/posthog/hogql_queries/ai/team_taxonomy_query_runner.py @@ -0,0 +1,74 @@ +from datetime import datetime +from typing import Optional + +from posthog.caching.utils import ThresholdMode, is_stale +from posthog.hogql import ast +from posthog.hogql.parser import parse_select +from posthog.hogql.printer import to_printed_hogql +from posthog.hogql.query import execute_hogql_query +from posthog.hogql_queries.query_runner import QueryRunner +from posthog.schema import ( + CachedTeamTaxonomyQueryResponse, + TeamTaxonomyItem, + TeamTaxonomyQuery, + TeamTaxonomyQueryResponse, +) + + +class TeamTaxonomyQueryRunner(QueryRunner): + """ + Calculates the top events for a team sorted by count. The EventDefinition model doesn't store the count of events, + so this query mitigates that. + """ + + query: TeamTaxonomyQuery + response: TeamTaxonomyQueryResponse + cached_response: CachedTeamTaxonomyQueryResponse + + def calculate(self): + query = self.to_query() + hogql = to_printed_hogql(query, self.team) + + response = execute_hogql_query( + query_type="TeamTaxonomyQuery", + query=query, + team=self.team, + timings=self.timings, + modifiers=self.modifiers, + limit_context=self.limit_context, + ) + + results: list[TeamTaxonomyItem] = [] + for event, count in response.results: + results.append(TeamTaxonomyItem(event=event, count=count)) + + return TeamTaxonomyQueryResponse( + results=results, timings=response.timings, hogql=hogql, modifiers=self.modifiers + ) + + def to_query(self) -> ast.SelectQuery | ast.SelectUnionQuery: + query = parse_select( + """ + SELECT + event, + count() as count + FROM events + WHERE + timestamp >= now () - INTERVAL 30 DAY + GROUP BY + event + ORDER BY + count DESC + """ + ) + + return query + + def _is_stale(self, last_refresh: Optional[datetime], lazy: bool = False) -> bool: + """ + Despite the lazy mode, it caches for an hour by default. We don't want frequent updates here. + """ + return is_stale(self.team, date_to=None, interval=None, last_refresh=last_refresh, mode=ThresholdMode.AI) + + def cache_target_age(self, last_refresh: Optional[datetime], lazy: bool = False) -> Optional[datetime]: + return None diff --git a/posthog/hogql_queries/ai/test/__init__.py b/posthog/hogql_queries/ai/test/__init__.py new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/posthog/hogql_queries/ai/test/__snapshots__/test_event_taxonomy_query_runner.ambr b/posthog/hogql_queries/ai/test/__snapshots__/test_event_taxonomy_query_runner.ambr new file mode 100644 index 0000000000000..d7efa8daa59ce --- /dev/null +++ b/posthog/hogql_queries/ai/test/__snapshots__/test_event_taxonomy_query_runner.ambr @@ -0,0 +1,27 @@ +# serializer version: 1 +# name: TestEventTaxonomyQueryRunner.test_event_taxonomy_query_runner + ''' + SELECT key, + arraySlice(arrayDistinct(groupArray(value)), 1, 5) AS + values, + count(DISTINCT value) AS total_count + FROM + (SELECT JSONExtractKeysAndValues(events.properties, 'String') AS kv + FROM events + WHERE and(equals(events.team_id, 2), greaterOrEquals(toTimeZone(events.timestamp, 'UTC'), minus(now64(6, 'UTC'), toIntervalDay(30))), equals(events.event, 'event1')) + ORDER BY toTimeZone(events.timestamp, 'UTC') DESC + LIMIT 100) ARRAY + JOIN (kv).1 AS key, + (kv).2 AS value + WHERE not(match(key, '(\\$set|\\$time|\\$set_once|\\$sent_at|\\$ip|__|phjs|survey_dismissed|survey_responded|partial_filter_chosen|changed_action|window-id|changed_event|partial_filter)')) + GROUP BY key + ORDER BY total_count DESC + LIMIT 100 SETTINGS readonly=2, + max_execution_time=60, + allow_experimental_object_type=1, + format_csv_allow_double_quotes=0, + max_ast_elements=4000000, + max_expanded_ast_elements=4000000, + max_bytes_before_external_group_by=0 + ''' +# --- diff --git a/posthog/hogql_queries/ai/test/__snapshots__/test_team_taxonomy_query_runner.ambr b/posthog/hogql_queries/ai/test/__snapshots__/test_team_taxonomy_query_runner.ambr new file mode 100644 index 0000000000000..bc45bf799e351 --- /dev/null +++ b/posthog/hogql_queries/ai/test/__snapshots__/test_team_taxonomy_query_runner.ambr @@ -0,0 +1,18 @@ +# serializer version: 1 +# name: TestTeamTaxonomyQueryRunner.test_taxonomy_query_runner + ''' + SELECT events.event AS event, + count() AS count + FROM events + WHERE and(equals(events.team_id, 2), greaterOrEquals(toTimeZone(events.timestamp, 'UTC'), minus(now64(6, 'UTC'), toIntervalDay(30)))) + GROUP BY events.event + ORDER BY count DESC + LIMIT 100 SETTINGS readonly=2, + max_execution_time=60, + allow_experimental_object_type=1, + format_csv_allow_double_quotes=0, + max_ast_elements=4000000, + max_expanded_ast_elements=4000000, + max_bytes_before_external_group_by=0 + ''' +# --- diff --git a/posthog/hogql_queries/ai/test/test_event_taxonomy_query_runner.py b/posthog/hogql_queries/ai/test/test_event_taxonomy_query_runner.py new file mode 100644 index 0000000000000..6807d1d8c53e8 --- /dev/null +++ b/posthog/hogql_queries/ai/test/test_event_taxonomy_query_runner.py @@ -0,0 +1,240 @@ +from datetime import timedelta + +from django.test import override_settings +from django.utils import timezone +from freezegun import freeze_time + +from posthog.hogql_queries.ai.event_taxonomy_query_runner import EventTaxonomyQueryRunner +from posthog.schema import CachedEventTaxonomyQueryResponse, EventTaxonomyQuery +from posthog.test.base import ( + APIBaseTest, + ClickhouseTestMixin, + _create_event, + _create_person, + flush_persons_and_events, + snapshot_clickhouse_queries, +) + + +@override_settings(IN_UNIT_TESTING=True) +class TestEventTaxonomyQueryRunner(ClickhouseTestMixin, APIBaseTest): + @snapshot_clickhouse_queries + def test_event_taxonomy_query_runner(self): + _create_person( + distinct_ids=["person1"], + properties={"email": "person1@example.com"}, + team=self.team, + ) + _create_event( + event="event1", + distinct_id="person1", + properties={"$browser": "Chrome", "$country": "US"}, + team=self.team, + ) + _create_event( + event="event1", + distinct_id="person1", + properties={"$browser": "Safari", "$country": "UK"}, + team=self.team, + ) + _create_event( + event="event1", + distinct_id="person1", + properties={"$browser": "Firefox", "$country": "US"}, + team=self.team, + ) + _create_event( + event="event1", + distinct_id="person1", + properties={"$browser": "Mobile Safari", "$country": "UK"}, + team=self.team, + ) + _create_event( + event="event1", + distinct_id="person1", + properties={"$browser": "Netscape", "$country": "US"}, + team=self.team, + ) + _create_event( + event="event1", + distinct_id="person1", + properties={"$browser": "Mobile Chrome", "$country": "UK"}, + team=self.team, + ) + + response = EventTaxonomyQueryRunner(team=self.team, query=EventTaxonomyQuery(event="event1")).calculate() + self.assertEqual(len(response.results), 2) + self.assertEqual(response.results[0].property, "$browser") + self.assertEqual( + response.results[0].sample_values, + [ + "Mobile Chrome", + "Netscape", + "Mobile Safari", + "Firefox", + "Safari", + ], + ) + self.assertEqual(response.results[0].sample_count, 6) + self.assertEqual(response.results[1].property, "$country") + self.assertEqual(response.results[1].sample_values, ["UK", "US"]) + self.assertEqual(response.results[1].sample_count, 2) + + def test_event_taxonomy_query_filters_by_event(self): + _create_person( + distinct_ids=["person1"], + properties={"email": "person1@example.com"}, + team=self.team, + ) + _create_event( + event="event1", + distinct_id="person1", + properties={"$browser": "Chrome", "$country": "US"}, + team=self.team, + ) + _create_event( + event="event1", + distinct_id="person1", + properties={"$browser": "Chrome", "$country": "UK"}, + team=self.team, + ) + _create_event( + event="event2", + distinct_id="person1", + properties={"$browser": "Safari", "$country": "UK"}, + team=self.team, + ) + + response = EventTaxonomyQueryRunner(team=self.team, query=EventTaxonomyQuery(event="event1")).calculate() + self.assertEqual(len(response.results), 2) + self.assertEqual(response.results[0].property, "$country") + self.assertEqual(response.results[0].sample_values, ["UK", "US"]) + self.assertEqual(response.results[0].sample_count, 2) + self.assertEqual(response.results[1].property, "$browser") + self.assertEqual(response.results[1].sample_values, ["Chrome"]) + self.assertEqual(response.results[1].sample_count, 1) + + def test_event_taxonomy_query_excludes_properties(self): + _create_person( + distinct_ids=["person1"], + properties={"email": "person1@example.com"}, + team=self.team, + ) + _create_event( + event="event1", + distinct_id="person1", + properties={"$browser__name": "Chrome", "$country": "US"}, + team=self.team, + ) + _create_event( + event="event1", + distinct_id="person1", + properties={"$set": "data", "$set_once": "data"}, + team=self.team, + ) + + response = EventTaxonomyQueryRunner(team=self.team, query=EventTaxonomyQuery(event="event1")).calculate() + self.assertEqual(len(response.results), 1) + self.assertEqual(response.results[0].property, "$country") + self.assertEqual(response.results[0].sample_values, ["US"]) + self.assertEqual(response.results[0].sample_count, 1) + + def test_event_taxonomy_includes_properties_from_multiple_persons(self): + _create_person( + distinct_ids=["person1"], + properties={"email": "person1@example.com"}, + team=self.team, + ) + _create_person( + distinct_ids=["person2"], + properties={"email": "person1@example.com"}, + team=self.team, + ) + _create_event( + event="event1", + distinct_id="person1", + properties={"$browser": "Chrome", "$country": "US"}, + team=self.team, + ) + _create_event( + event="event1", + distinct_id="person2", + properties={"$browser": "Chrome", "$screen": "1024x768"}, + team=self.team, + ) + + response = EventTaxonomyQueryRunner(team=self.team, query=EventTaxonomyQuery(event="event1")).calculate() + results = sorted(response.results, key=lambda x: x.property) + self.assertEqual(len(results), 3) + self.assertEqual(results[0].property, "$browser") + self.assertEqual(results[0].sample_values, ["Chrome"]) + self.assertEqual(results[0].sample_count, 1) + self.assertEqual(results[1].property, "$country") + self.assertEqual(results[1].sample_values, ["US"]) + self.assertEqual(results[1].sample_count, 1) + self.assertEqual(results[2].property, "$screen") + self.assertEqual(results[2].sample_values, ["1024x768"]) + self.assertEqual(results[2].sample_count, 1) + + def test_is_stale(self): + date = timezone.now() + runner = EventTaxonomyQueryRunner(team=self.team, query=EventTaxonomyQuery(event="event1")) + self.assertFalse(runner._is_stale(last_refresh=date, lazy=False)) + self.assertFalse(runner._is_stale(last_refresh=date, lazy=True)) + self.assertFalse(runner._is_stale(last_refresh=date - timedelta(minutes=15), lazy=False)) + self.assertFalse(runner._is_stale(last_refresh=date - timedelta(minutes=15), lazy=True)) + self.assertFalse(runner._is_stale(last_refresh=date - timedelta(minutes=59), lazy=True)) + self.assertFalse(runner._is_stale(last_refresh=date - timedelta(minutes=59), lazy=False)) + self.assertTrue(runner._is_stale(last_refresh=date - timedelta(minutes=60), lazy=True)) + self.assertTrue(runner._is_stale(last_refresh=date - timedelta(minutes=60), lazy=False)) + + def test_caching(self): + now = timezone.now() + + with freeze_time(now): + _create_person( + distinct_ids=["person1"], + properties={"email": "person1@example.com"}, + team=self.team, + ) + _create_event( + event="event1", + distinct_id="person1", + team=self.team, + ) + + runner = EventTaxonomyQueryRunner(team=self.team, query=EventTaxonomyQuery(event="event1")) + response = runner.run() + + assert isinstance(response, CachedEventTaxonomyQueryResponse) + self.assertEqual(len(response.results), 0) + + key = response.cache_key + _create_event( + event="event1", + distinct_id="person1", + properties={"$browser": "Chrome"}, + team=self.team, + ) + flush_persons_and_events() + + runner = EventTaxonomyQueryRunner(team=self.team, query=EventTaxonomyQuery(event="event1")) + response = runner.run() + + assert isinstance(response, CachedEventTaxonomyQueryResponse) + self.assertEqual(response.cache_key, key) + self.assertEqual(len(response.results), 0) + + with freeze_time(now + timedelta(minutes=59)): + runner = EventTaxonomyQueryRunner(team=self.team, query=EventTaxonomyQuery(event="event1")) + response = runner.run() + + assert isinstance(response, CachedEventTaxonomyQueryResponse) + self.assertEqual(len(response.results), 0) + + with freeze_time(now + timedelta(minutes=61)): + runner = EventTaxonomyQueryRunner(team=self.team, query=EventTaxonomyQuery(event="event1")) + response = runner.run() + + assert isinstance(response, CachedEventTaxonomyQueryResponse) + self.assertEqual(len(response.results), 1) diff --git a/posthog/hogql_queries/ai/test/test_team_taxonomy_query_runner.py b/posthog/hogql_queries/ai/test/test_team_taxonomy_query_runner.py new file mode 100644 index 0000000000000..8c29090224e1e --- /dev/null +++ b/posthog/hogql_queries/ai/test/test_team_taxonomy_query_runner.py @@ -0,0 +1,114 @@ +from datetime import timedelta + +from django.test import override_settings +from django.utils import timezone +from freezegun import freeze_time + +from posthog.hogql_queries.ai.team_taxonomy_query_runner import TeamTaxonomyQueryRunner +from posthog.schema import CachedTeamTaxonomyQueryResponse, TeamTaxonomyQuery +from posthog.test.base import ( + APIBaseTest, + ClickhouseTestMixin, + _create_event, + _create_person, + flush_persons_and_events, + snapshot_clickhouse_queries, +) + + +@override_settings(IN_UNIT_TESTING=True) +class TestTeamTaxonomyQueryRunner(ClickhouseTestMixin, APIBaseTest): + @snapshot_clickhouse_queries + def test_taxonomy_query_runner(self): + _create_person( + distinct_ids=["person1"], + properties={"email": "person1@example.com"}, + team=self.team, + ) + _create_event( + event="event1", + distinct_id="person1", + properties={"$browser": "Chrome", "$country": "US"}, + team=self.team, + ) + _create_event( + event="event2", + distinct_id="person1", + properties={"$browser": "Chrome", "$country": "US"}, + team=self.team, + ) + _create_event( + event="event1", + distinct_id="person1", + properties={"$browser": "Chrome", "$country": "US"}, + team=self.team, + ) + + results = TeamTaxonomyQueryRunner(team=self.team, query=TeamTaxonomyQuery()).calculate() + self.assertEqual(len(results.results), 2) + self.assertEqual(results.results[0].event, "event1") + self.assertEqual(results.results[0].count, 2) + self.assertEqual(results.results[1].event, "event2") + self.assertEqual(results.results[1].count, 1) + + def test_is_stale(self): + date = timezone.now() + runner = TeamTaxonomyQueryRunner(team=self.team, query=TeamTaxonomyQuery()) + self.assertFalse(runner._is_stale(last_refresh=date, lazy=False)) + self.assertFalse(runner._is_stale(last_refresh=date, lazy=True)) + self.assertFalse(runner._is_stale(last_refresh=date - timedelta(minutes=15), lazy=False)) + self.assertFalse(runner._is_stale(last_refresh=date - timedelta(minutes=15), lazy=True)) + self.assertFalse(runner._is_stale(last_refresh=date - timedelta(minutes=59), lazy=True)) + self.assertFalse(runner._is_stale(last_refresh=date - timedelta(minutes=59), lazy=False)) + self.assertTrue(runner._is_stale(last_refresh=date - timedelta(minutes=60), lazy=True)) + self.assertTrue(runner._is_stale(last_refresh=date - timedelta(minutes=60), lazy=False)) + + def test_caching(self): + now = timezone.now() + + with freeze_time(now): + _create_person( + distinct_ids=["person1"], + properties={"email": "person1@example.com"}, + team=self.team, + ) + _create_event( + event="event1", + distinct_id="person1", + team=self.team, + ) + + runner = TeamTaxonomyQueryRunner(team=self.team, query=TeamTaxonomyQuery()) + response = runner.run() + + assert isinstance(response, CachedTeamTaxonomyQueryResponse) + self.assertEqual(len(response.results), 1) + + key = response.cache_key + _create_event( + event="event2", + distinct_id="person1", + team=self.team, + ) + flush_persons_and_events() + + runner = TeamTaxonomyQueryRunner(team=self.team, query=TeamTaxonomyQuery()) + response = runner.run() + + assert isinstance(response, CachedTeamTaxonomyQueryResponse) + self.assertEqual(response.cache_key, key) + self.assertEqual(len(response.results), 1) + + with freeze_time(now + timedelta(minutes=59)): + runner = TeamTaxonomyQueryRunner(team=self.team, query=TeamTaxonomyQuery()) + response = runner.run() + + assert isinstance(response, CachedTeamTaxonomyQueryResponse) + self.assertEqual(len(response.results), 1) + + with freeze_time(now + timedelta(minutes=61)): + runner = TeamTaxonomyQueryRunner(team=self.team, query=TeamTaxonomyQuery()) + response = runner.run() + + assert isinstance(response, CachedTeamTaxonomyQueryResponse) + self.assertEqual(len(response.results), 2) diff --git a/posthog/hogql_queries/error_tracking_query_runner.py b/posthog/hogql_queries/error_tracking_query_runner.py index 49f2a19023aa5..dc40e966d2eb5 100644 --- a/posthog/hogql_queries/error_tracking_query_runner.py +++ b/posthog/hogql_queries/error_tracking_query_runner.py @@ -1,3 +1,4 @@ +import re from posthog.hogql import ast from posthog.hogql.constants import LimitContext from posthog.hogql_queries.insights.paginators import HogQLHasMorePaginator @@ -130,31 +131,49 @@ def where(self): if self.query.searchQuery: # TODO: Refine this so it only searches the frames inside $exception_list - # TODO: Split out spaces and search for each word separately - # TODO: Add support for searching for specific properties + # TODO: We'd eventually need a more efficient searching strategy # TODO: Add fuzzy search support - props_to_search = ["$exception_list", "$exception_stack_trace_raw", "$exception_type", "$exception_message"] - or_exprs: list[ast.Expr] = [] - for prop in props_to_search: - or_exprs.append( - ast.CompareOperation( - op=ast.CompareOperationOp.Gt, - left=ast.Call( - name="position", - args=[ - ast.Call(name="lower", args=[ast.Field(chain=["properties", prop])]), - ast.Call(name="lower", args=[ast.Constant(value=self.query.searchQuery)]), - ], - ), - right=ast.Constant(value=0), + + # first parse the search query to split it into words, except for quoted strings + # then search for each word in the exception properties + tokens = search_tokenizer(self.query.searchQuery) + and_exprs: list[ast.Expr] = [] + + if len(tokens) > 10: + raise ValueError("Too many search tokens") + + for token in tokens: + if not token: + continue + + or_exprs: list[ast.Expr] = [] + props_to_search = [ + "$exception_list", + "$exception_stack_trace_raw", + "$exception_type", + "$exception_message", + ] + for prop in props_to_search: + or_exprs.append( + ast.CompareOperation( + op=ast.CompareOperationOp.Gt, + left=ast.Call( + name="position", + args=[ + ast.Call(name="lower", args=[ast.Field(chain=["properties", prop])]), + ast.Call(name="lower", args=[ast.Constant(value=token)]), + ], + ), + right=ast.Constant(value=0), + ) ) - ) - exprs.append( - ast.Or( - exprs=or_exprs, + and_exprs.append( + ast.Or( + exprs=or_exprs, + ) ) - ) + exprs.append(ast.And(exprs=and_exprs)) return ast.And(exprs=exprs) @@ -254,7 +273,6 @@ def error_tracking_groups(self): queryset = ErrorTrackingGroup.objects.filter(team=self.team) # :TRICKY: Ideally we'd have no null characters in the fingerprint, but if something made it into the pipeline with null characters # (because rest of the system supports it), try cleaning it up here. Make sure this cleaning is consistent with the rest of the system. - # This does mean we'll not match with this ErrorTrackingGroup cleaned_fingerprint = [part.replace("\x00", "\ufffd") for part in self.query.fingerprint or []] queryset = ( queryset.filter(fingerprint=cleaned_fingerprint) @@ -264,3 +282,14 @@ def error_tracking_groups(self): queryset = queryset.filter(assignee=self.query.assignee) if self.query.assignee else queryset groups = queryset.values("fingerprint", "merged_fingerprints", "status", "assignee") return {str(item["fingerprint"]): item for item in groups} + + +def search_tokenizer(query: str) -> list[str]: + # parse the search query to split it into words, except for quoted strings. Strip quotes from quoted strings. + # Example: 'This is a "quoted string" and this is \'another one\' with some words' + # Output: ['This', 'is', 'a', 'quoted string', 'and', 'this', 'is', 'another one', 'with', 'some', 'words'] + # This doesn't handle nested quotes, and some complex edge cases, but we don't really need that for now. + # If requirements do change, consider using a proper parser like `pyparsing` + pattern = r'"[^"]*"|\'[^\']*\'|\S+' + tokens = re.findall(pattern, query) + return [token.strip("'\"") for token in tokens] diff --git a/posthog/hogql_queries/insights/funnels/base.py b/posthog/hogql_queries/insights/funnels/base.py index 8066ada3c642f..cf6836a4dd168 100644 --- a/posthog/hogql_queries/insights/funnels/base.py +++ b/posthog/hogql_queries/insights/funnels/base.py @@ -25,7 +25,6 @@ from posthog.queries.util import correct_result_for_sampling from posthog.schema import ( ActionsNode, - BaseMathType, BreakdownAttributionType, BreakdownType, DataWarehouseNode, @@ -34,6 +33,7 @@ FunnelTimeToConvertResults, FunnelVizType, FunnelExclusionEventsNode, + FunnelMathType, ) from posthog.types import EntityNode, ExclusionEntityNode @@ -697,10 +697,14 @@ def _build_step_query( filter_expr = property_to_expr(entity.properties, self.context.team) filters.append(filter_expr) - if entity.math == BaseMathType.FIRST_TIME_FOR_USER: + if entity.math == FunnelMathType.FIRST_TIME_FOR_USER: subquery = FirstTimeForUserAggregationQuery(self.context, filter_expr, event_expr).to_query() first_time_filter = parse_expr("e.uuid IN {subquery}", placeholders={"subquery": subquery}) return ast.And(exprs=[*filters, first_time_filter]) + elif entity.math == FunnelMathType.FIRST_TIME_FOR_USER_WITH_FILTERS: + subquery = FirstTimeForUserAggregationQuery(self.context, ast.Constant(value=1), filter_expr).to_query() + first_time_filter = parse_expr("e.uuid IN {subquery}", placeholders={"subquery": subquery}) + return ast.And(exprs=[*filters, first_time_filter]) elif len(filters) > 1: return ast.And(exprs=filters) return filters[0] diff --git a/posthog/hogql_queries/insights/funnels/funnel_udf.py b/posthog/hogql_queries/insights/funnels/funnel_udf.py index a49cbd09d4984..e867975d04f45 100644 --- a/posthog/hogql_queries/insights/funnels/funnel_udf.py +++ b/posthog/hogql_queries/insights/funnels/funnel_udf.py @@ -39,13 +39,15 @@ def conversion_window_limit(self) -> int: # This is used to reduce the number of events we look at in strict funnels # We remove a non-matching event if there was already one before it (that don't have the same timestamp) + # arrayRotateRight turns [1,2,3] into [3,1,2] + # For some reason, this uses much less memory than using indexing in clickhouse to check the previous element def _array_filter(self): if self.context.funnelsFilter.funnelOrderType == "strict": return f""" arrayFilter( - (x, i) -> not (isNotNull(events_array[i-1]) and empty(x.4) and empty(events_array[i-1].4) and x.1 > events_array[i-1].1), + (x, x2) -> not (empty(x.4) and empty(x2.4) and x.1 > x2.1), events_array, - arrayEnumerate(events_array)) + arrayRotateRight(events_array, 1)) """ return "events_array" diff --git a/posthog/hogql_queries/insights/funnels/test/__snapshots__/test_funnel_correlation_actors_udf.ambr b/posthog/hogql_queries/insights/funnels/test/__snapshots__/test_funnel_correlation_actors_udf.ambr index 2637bfa739956..ed1a6761a9584 100644 --- a/posthog/hogql_queries/insights/funnels/test/__snapshots__/test_funnel_correlation_actors_udf.ambr +++ b/posthog/hogql_queries/insights/funnels/test/__snapshots__/test_funnel_correlation_actors_udf.ambr @@ -389,7 +389,8 @@ breakdown AS final_prop FROM (SELECT arraySort(t -> t.1, groupArray(tuple(accurateCastOrNull(timestamp, 'Float64'), uuid, [], arrayFilter(x -> ifNull(notEquals(x, 0), 1), [multiply(1, step_0), multiply(2, step_1)])))) AS events_array, - arrayJoin(aggregate_funnel_array(2, 1209600, 'first_touch', 'strict', [[]], arrayFilter((x, i) -> not(and(isNotNull(events_array[minus(i, 1)]), empty(x.4), empty((events_array[minus(i, 1)]).4), ifNull(greater(x.1, (events_array[minus(i, 1)]).1), 0))), events_array, arrayEnumerate(events_array)))) AS af_tuple, + + arrayJoin(aggregate_funnel_array_v0(2, 1209600, 'first_touch', 'strict', [[]], arrayFilter((x, x2) -> not(and(empty(x.4), empty(x2.4), ifNull(greater(x.1, x2.1), 0))), events_array, arrayRotateRight(events_array, 1)))) AS af_tuple, af_tuple.1 AS step_reached, plus(af_tuple.1, 1) AS steps, af_tuple.2 AS breakdown, @@ -446,7 +447,8 @@ first_timestamp FROM (SELECT arraySort(t -> t.1, groupArray(tuple(accurateCastOrNull(timestamp, 'Float64'), uuid, [], arrayFilter(x -> ifNull(notEquals(x, 0), 1), [multiply(1, step_0), multiply(2, step_1)])))) AS events_array, - arrayJoin(aggregate_funnel_array(2, 1209600, 'first_touch', 'strict', [[]], arrayFilter((x, i) -> not(and(isNotNull(events_array[minus(i, 1)]), empty(x.4), empty((events_array[minus(i, 1)]).4), ifNull(greater(x.1, (events_array[minus(i, 1)]).1), 0))), events_array, arrayEnumerate(events_array)))) AS af_tuple, + + arrayJoin(aggregate_funnel_array_v0(2, 1209600, 'first_touch', 'strict', [[]], arrayFilter((x, x2) -> not(and(empty(x.4), empty(x2.4), ifNull(greater(x.1, x2.1), 0))), events_array, arrayRotateRight(events_array, 1)))) AS af_tuple, af_tuple.1 AS step_reached, plus(af_tuple.1, 1) AS steps, af_tuple.2 AS breakdown, @@ -500,7 +502,7 @@ FROM (SELECT aggregation_target AS actor_id, matched_events_array[plus(step_reached, 1)] AS matching_events, (matched_events_array[1][1]).1 AS timestamp, nullIf((matched_events_array[2][1]).1, 0) AS final_timestamp, (matched_events_array[1][1]).1 AS first_timestamp, steps AS steps, final_timestamp, first_timestamp FROM - (SELECT arraySort(t -> t.1, groupArray(tuple(accurateCastOrNull(timestamp, 'Float64'), uuid, [], arrayFilter(x -> ifNull(notEquals(x, 0), 1), [multiply(1, step_0), multiply(2, step_1)])))) AS events_array, arrayJoin(aggregate_funnel_array(2, 1209600, 'first_touch', 'strict', [[]], arrayFilter((x, i) -> not(and(isNotNull(events_array[minus(i, 1)]), empty(x.4), empty((events_array[minus(i, 1)]).4), ifNull(greater(x.1, (events_array[minus(i, 1)]).1), 0))), events_array, arrayEnumerate(events_array)))) AS af_tuple, af_tuple.1 AS step_reached, plus(af_tuple.1, 1) AS steps, af_tuple.2 AS breakdown, af_tuple.3 AS timings, af_tuple.4 AS matched_event_uuids_array_array, groupArray(tuple(timestamp, uuid, `$session_id`, `$window_id`)) AS user_events, mapFromArrays(arrayMap(x -> x.2, user_events), user_events) AS user_events_map, arrayMap(matched_event_uuids_array -> arrayMap(event_uuid -> user_events_map[event_uuid], arrayDistinct(matched_event_uuids_array)), matched_event_uuids_array_array) AS matched_events_array, aggregation_target AS aggregation_target + (SELECT arraySort(t -> t.1, groupArray(tuple(accurateCastOrNull(timestamp, 'Float64'), uuid, [], arrayFilter(x -> ifNull(notEquals(x, 0), 1), [multiply(1, step_0), multiply(2, step_1)])))) AS events_array, arrayJoin(aggregate_funnel_array_v0(2, 1209600, 'first_touch', 'strict', [[]], arrayFilter((x, x2) -> not(and(empty(x.4), empty(x2.4), ifNull(greater(x.1, x2.1), 0))), events_array, arrayRotateRight(events_array, 1)))) AS af_tuple, af_tuple.1 AS step_reached, plus(af_tuple.1, 1) AS steps, af_tuple.2 AS breakdown, af_tuple.3 AS timings, af_tuple.4 AS matched_event_uuids_array_array, groupArray(tuple(timestamp, uuid, `$session_id`, `$window_id`)) AS user_events, mapFromArrays(arrayMap(x -> x.2, user_events), user_events) AS user_events_map, arrayMap(matched_event_uuids_array -> arrayMap(event_uuid -> user_events_map[event_uuid], arrayDistinct(matched_event_uuids_array)), matched_event_uuids_array_array) AS matched_events_array, aggregation_target AS aggregation_target FROM (SELECT toTimeZone(e.timestamp, 'UTC') AS timestamp, if(not(empty(e__override.distinct_id)), e__override.person_id, e.person_id) AS aggregation_target, e.uuid AS uuid, e.`$session_id` AS `$session_id`, e.`$window_id` AS `$window_id`, if(equals(e.event, '$pageview'), 1, 0) AS step_0, if(equals(e.event, 'insight analyzed'), 1, 0) AS step_1 FROM events AS e @@ -573,7 +575,7 @@ first_timestamp FROM (SELECT arraySort(t -> t.1, groupArray(tuple(accurateCastOrNull(timestamp, 'Float64'), uuid, [], arrayFilter(x -> ifNull(notEquals(x, 0), 1), [multiply(1, step_0), multiply(2, step_1)])))) AS events_array, - arrayJoin(aggregate_funnel_array(2, 1209600, 'first_touch', 'strict', [[]], arrayFilter((x, i) -> not(and(isNotNull(events_array[minus(i, 1)]), empty(x.4), empty((events_array[minus(i, 1)]).4), ifNull(greater(x.1, (events_array[minus(i, 1)]).1), 0))), events_array, arrayEnumerate(events_array)))) AS af_tuple, + arrayJoin(aggregate_funnel_array_v0(2, 1209600, 'first_touch', 'strict', [[]], arrayFilter((x, x2) -> not(and(empty(x.4), empty(x2.4), ifNull(greater(x.1, x2.1), 0))), events_array, arrayRotateRight(events_array, 1)))) AS af_tuple, af_tuple.1 AS step_reached, plus(af_tuple.1, 1) AS steps, af_tuple.2 AS breakdown, @@ -627,7 +629,7 @@ FROM (SELECT aggregation_target AS actor_id, matched_events_array[plus(step_reached, 1)] AS matching_events, (matched_events_array[1][1]).1 AS timestamp, nullIf((matched_events_array[2][1]).1, 0) AS final_timestamp, (matched_events_array[1][1]).1 AS first_timestamp, steps AS steps, final_timestamp, first_timestamp FROM - (SELECT arraySort(t -> t.1, groupArray(tuple(accurateCastOrNull(timestamp, 'Float64'), uuid, [], arrayFilter(x -> ifNull(notEquals(x, 0), 1), [multiply(1, step_0), multiply(2, step_1)])))) AS events_array, arrayJoin(aggregate_funnel_array(2, 1209600, 'first_touch', 'strict', [[]], arrayFilter((x, i) -> not(and(isNotNull(events_array[minus(i, 1)]), empty(x.4), empty((events_array[minus(i, 1)]).4), ifNull(greater(x.1, (events_array[minus(i, 1)]).1), 0))), events_array, arrayEnumerate(events_array)))) AS af_tuple, af_tuple.1 AS step_reached, plus(af_tuple.1, 1) AS steps, af_tuple.2 AS breakdown, af_tuple.3 AS timings, af_tuple.4 AS matched_event_uuids_array_array, groupArray(tuple(timestamp, uuid, `$session_id`, `$window_id`)) AS user_events, mapFromArrays(arrayMap(x -> x.2, user_events), user_events) AS user_events_map, arrayMap(matched_event_uuids_array -> arrayMap(event_uuid -> user_events_map[event_uuid], arrayDistinct(matched_event_uuids_array)), matched_event_uuids_array_array) AS matched_events_array, aggregation_target AS aggregation_target + (SELECT arraySort(t -> t.1, groupArray(tuple(accurateCastOrNull(timestamp, 'Float64'), uuid, [], arrayFilter(x -> ifNull(notEquals(x, 0), 1), [multiply(1, step_0), multiply(2, step_1)])))) AS events_array, arrayJoin(aggregate_funnel_array_v0(2, 1209600, 'first_touch', 'strict', [[]], arrayFilter((x, x2) -> not(and(empty(x.4), empty(x2.4), ifNull(greater(x.1, x2.1), 0))), events_array, arrayRotateRight(events_array, 1)))) AS af_tuple, af_tuple.1 AS step_reached, plus(af_tuple.1, 1) AS steps, af_tuple.2 AS breakdown, af_tuple.3 AS timings, af_tuple.4 AS matched_event_uuids_array_array, groupArray(tuple(timestamp, uuid, `$session_id`, `$window_id`)) AS user_events, mapFromArrays(arrayMap(x -> x.2, user_events), user_events) AS user_events_map, arrayMap(matched_event_uuids_array -> arrayMap(event_uuid -> user_events_map[event_uuid], arrayDistinct(matched_event_uuids_array)), matched_event_uuids_array_array) AS matched_events_array, aggregation_target AS aggregation_target FROM (SELECT toTimeZone(e.timestamp, 'UTC') AS timestamp, if(not(empty(e__override.distinct_id)), e__override.person_id, e.person_id) AS aggregation_target, e.uuid AS uuid, e.`$session_id` AS `$session_id`, e.`$window_id` AS `$window_id`, if(equals(e.event, '$pageview'), 1, 0) AS step_0, if(equals(e.event, 'insight analyzed'), 1, 0) AS step_1 FROM events AS e diff --git a/posthog/hogql_queries/insights/funnels/test/__snapshots__/test_funnel_strict_persons_udf.ambr b/posthog/hogql_queries/insights/funnels/test/__snapshots__/test_funnel_strict_persons_udf.ambr index 723a136238507..031aa7363bed0 100644 --- a/posthog/hogql_queries/insights/funnels/test/__snapshots__/test_funnel_strict_persons_udf.ambr +++ b/posthog/hogql_queries/insights/funnels/test/__snapshots__/test_funnel_strict_persons_udf.ambr @@ -9,7 +9,7 @@ matched_events_array[1] AS matching_events FROM (SELECT arraySort(t -> t.1, groupArray(tuple(accurateCastOrNull(timestamp, 'Float64'), uuid, [], arrayFilter(x -> ifNull(notEquals(x, 0), 1), [multiply(1, step_0), multiply(2, step_1), multiply(3, step_2)])))) AS events_array, - arrayJoin(aggregate_funnel_array(3, 1209600, 'first_touch', 'strict', [[]], arrayFilter((x, i) -> not(and(isNotNull(events_array[minus(i, 1)]), empty(x.4), empty((events_array[minus(i, 1)]).4), ifNull(greater(x.1, (events_array[minus(i, 1)]).1), 0))), events_array, arrayEnumerate(events_array)))) AS af_tuple, + arrayJoin(aggregate_funnel_array_v0(3, 1209600, 'first_touch', 'strict', [[]], arrayFilter((x, x2) -> not(and(empty(x.4), empty(x2.4), ifNull(greater(x.1, x2.1), 0))), events_array, arrayRotateRight(events_array, 1)))) AS af_tuple, af_tuple.1 AS step_reached, plus(af_tuple.1, 1) AS steps, af_tuple.2 AS breakdown, @@ -83,7 +83,7 @@ matched_events_array[2] AS matching_events FROM (SELECT arraySort(t -> t.1, groupArray(tuple(accurateCastOrNull(timestamp, 'Float64'), uuid, [], arrayFilter(x -> ifNull(notEquals(x, 0), 1), [multiply(1, step_0), multiply(2, step_1), multiply(3, step_2)])))) AS events_array, - arrayJoin(aggregate_funnel_array(3, 1209600, 'first_touch', 'strict', [[]], arrayFilter((x, i) -> not(and(isNotNull(events_array[minus(i, 1)]), empty(x.4), empty((events_array[minus(i, 1)]).4), ifNull(greater(x.1, (events_array[minus(i, 1)]).1), 0))), events_array, arrayEnumerate(events_array)))) AS af_tuple, + arrayJoin(aggregate_funnel_array_v0(3, 1209600, 'first_touch', 'strict', [[]], arrayFilter((x, x2) -> not(and(empty(x.4), empty(x2.4), ifNull(greater(x.1, x2.1), 0))), events_array, arrayRotateRight(events_array, 1)))) AS af_tuple, af_tuple.1 AS step_reached, plus(af_tuple.1, 1) AS steps, af_tuple.2 AS breakdown, @@ -157,7 +157,7 @@ matched_events_array[2] AS matching_events FROM (SELECT arraySort(t -> t.1, groupArray(tuple(accurateCastOrNull(timestamp, 'Float64'), uuid, [], arrayFilter(x -> ifNull(notEquals(x, 0), 1), [multiply(1, step_0), multiply(2, step_1), multiply(3, step_2)])))) AS events_array, - arrayJoin(aggregate_funnel_array(3, 1209600, 'first_touch', 'strict', [[]], arrayFilter((x, i) -> not(and(isNotNull(events_array[minus(i, 1)]), empty(x.4), empty((events_array[minus(i, 1)]).4), ifNull(greater(x.1, (events_array[minus(i, 1)]).1), 0))), events_array, arrayEnumerate(events_array)))) AS af_tuple, + arrayJoin(aggregate_funnel_array_v0(3, 1209600, 'first_touch', 'strict', [[]], arrayFilter((x, x2) -> not(and(empty(x.4), empty(x2.4), ifNull(greater(x.1, x2.1), 0))), events_array, arrayRotateRight(events_array, 1)))) AS af_tuple, af_tuple.1 AS step_reached, plus(af_tuple.1, 1) AS steps, af_tuple.2 AS breakdown, diff --git a/posthog/hogql_queries/insights/funnels/test/__snapshots__/test_funnel_strict_udf.ambr b/posthog/hogql_queries/insights/funnels/test/__snapshots__/test_funnel_strict_udf.ambr index 3d540f6981c87..d12ffea2f6bdd 100644 --- a/posthog/hogql_queries/insights/funnels/test/__snapshots__/test_funnel_strict_udf.ambr +++ b/posthog/hogql_queries/insights/funnels/test/__snapshots__/test_funnel_strict_udf.ambr @@ -15,7 +15,7 @@ if(ifNull(less(row_number, 25), 0), breakdown, ['Other']) AS final_prop FROM (SELECT arraySort(t -> t.1, groupArray(tuple(accurateCastOrNull(timestamp, 'Float64'), uuid, prop, arrayFilter(x -> ifNull(notEquals(x, 0), 1), [multiply(1, step_0), multiply(2, step_1)])))) AS events_array, - arrayJoin(aggregate_funnel_array(2, 1209600, 'first_touch', 'strict', groupUniqArray(prop), arrayFilter((x, i) -> not(and(isNotNull(events_array[minus(i, 1)]), empty(x.4), empty((events_array[minus(i, 1)]).4), ifNull(greater(x.1, (events_array[minus(i, 1)]).1), 0))), events_array, arrayEnumerate(events_array)))) AS af_tuple, + arrayJoin(aggregate_funnel_array_v0(2, 1209600, 'first_touch', 'strict', groupUniqArray(prop), arrayFilter((x, x2) -> not(and(empty(x.4), empty(x2.4), ifNull(greater(x.1, x2.1), 0))), events_array, arrayRotateRight(events_array, 1)))) AS af_tuple, af_tuple.1 AS step_reached, plus(af_tuple.1, 1) AS steps, af_tuple.2 AS breakdown, @@ -84,7 +84,7 @@ if(ifNull(less(row_number, 25), 0), breakdown, ['Other']) AS final_prop FROM (SELECT arraySort(t -> t.1, groupArray(tuple(accurateCastOrNull(timestamp, 'Float64'), uuid, prop, arrayFilter(x -> ifNull(notEquals(x, 0), 1), [multiply(1, step_0), multiply(2, step_1)])))) AS events_array, - arrayJoin(aggregate_funnel_array(2, 1209600, 'step_1', 'strict', groupUniqArray(prop), arrayFilter((x, i) -> not(and(isNotNull(events_array[minus(i, 1)]), empty(x.4), empty((events_array[minus(i, 1)]).4), ifNull(greater(x.1, (events_array[minus(i, 1)]).1), 0))), events_array, arrayEnumerate(events_array)))) AS af_tuple, + arrayJoin(aggregate_funnel_array_v0(2, 1209600, 'step_1', 'strict', groupUniqArray(prop), arrayFilter((x, x2) -> not(and(empty(x.4), empty(x2.4), ifNull(greater(x.1, x2.1), 0))), events_array, arrayRotateRight(events_array, 1)))) AS af_tuple, af_tuple.1 AS step_reached, plus(af_tuple.1, 1) AS steps, af_tuple.2 AS breakdown, @@ -160,7 +160,7 @@ if(ifNull(less(row_number, 25), 0), breakdown, ['Other']) AS final_prop FROM (SELECT arraySort(t -> t.1, groupArray(tuple(accurateCastOrNull(timestamp, 'Float64'), uuid, prop, arrayFilter(x -> ifNull(notEquals(x, 0), 1), [multiply(1, step_0), multiply(2, step_1)])))) AS events_array, - arrayJoin(aggregate_funnel_array(2, 1209600, 'first_touch', 'strict', groupUniqArray(prop), arrayFilter((x, i) -> not(and(isNotNull(events_array[minus(i, 1)]), empty(x.4), empty((events_array[minus(i, 1)]).4), ifNull(greater(x.1, (events_array[minus(i, 1)]).1), 0))), events_array, arrayEnumerate(events_array)))) AS af_tuple, + arrayJoin(aggregate_funnel_array_v0(2, 1209600, 'first_touch', 'strict', groupUniqArray(prop), arrayFilter((x, x2) -> not(and(empty(x.4), empty(x2.4), ifNull(greater(x.1, x2.1), 0))), events_array, arrayRotateRight(events_array, 1)))) AS af_tuple, af_tuple.1 AS step_reached, plus(af_tuple.1, 1) AS steps, af_tuple.2 AS breakdown, @@ -234,7 +234,7 @@ if(ifNull(less(row_number, 25), 0), breakdown, 'Other') AS final_prop FROM (SELECT arraySort(t -> t.1, groupArray(tuple(accurateCastOrNull(timestamp, 'Float64'), uuid, prop, arrayFilter(x -> ifNull(notEquals(x, 0), 1), [multiply(1, step_0), multiply(2, step_1), multiply(3, step_2)])))) AS events_array, - arrayJoin(aggregate_funnel(3, 1209600, 'first_touch', 'strict', groupUniqArray(prop), arrayFilter((x, i) -> not(and(isNotNull(events_array[minus(i, 1)]), empty(x.4), empty((events_array[minus(i, 1)]).4), ifNull(greater(x.1, (events_array[minus(i, 1)]).1), 0))), events_array, arrayEnumerate(events_array)))) AS af_tuple, + arrayJoin(aggregate_funnel_v0(3, 1209600, 'first_touch', 'strict', groupUniqArray(prop), arrayFilter((x, x2) -> not(and(empty(x.4), empty(x2.4), ifNull(greater(x.1, x2.1), 0))), events_array, arrayRotateRight(events_array, 1)))) AS af_tuple, af_tuple.1 AS step_reached, plus(af_tuple.1, 1) AS steps, af_tuple.2 AS breakdown, @@ -311,7 +311,7 @@ if(ifNull(less(row_number, 25), 0), breakdown, 'Other') AS final_prop FROM (SELECT arraySort(t -> t.1, groupArray(tuple(accurateCastOrNull(timestamp, 'Float64'), uuid, prop, arrayFilter(x -> ifNull(notEquals(x, 0), 1), [multiply(1, step_0), multiply(2, step_1), multiply(3, step_2)])))) AS events_array, - arrayJoin(aggregate_funnel(3, 1209600, 'first_touch', 'strict', groupUniqArray(prop), arrayFilter((x, i) -> not(and(isNotNull(events_array[minus(i, 1)]), empty(x.4), empty((events_array[minus(i, 1)]).4), ifNull(greater(x.1, (events_array[minus(i, 1)]).1), 0))), events_array, arrayEnumerate(events_array)))) AS af_tuple, + arrayJoin(aggregate_funnel_v0(3, 1209600, 'first_touch', 'strict', groupUniqArray(prop), arrayFilter((x, x2) -> not(and(empty(x.4), empty(x2.4), ifNull(greater(x.1, x2.1), 0))), events_array, arrayRotateRight(events_array, 1)))) AS af_tuple, af_tuple.1 AS step_reached, plus(af_tuple.1, 1) AS steps, af_tuple.2 AS breakdown, @@ -388,7 +388,7 @@ if(ifNull(less(row_number, 25), 0), breakdown, 'Other') AS final_prop FROM (SELECT arraySort(t -> t.1, groupArray(tuple(accurateCastOrNull(timestamp, 'Float64'), uuid, prop, arrayFilter(x -> ifNull(notEquals(x, 0), 1), [multiply(1, step_0), multiply(2, step_1), multiply(3, step_2)])))) AS events_array, - arrayJoin(aggregate_funnel(3, 1209600, 'first_touch', 'strict', groupUniqArray(prop), arrayFilter((x, i) -> not(and(isNotNull(events_array[minus(i, 1)]), empty(x.4), empty((events_array[minus(i, 1)]).4), ifNull(greater(x.1, (events_array[minus(i, 1)]).1), 0))), events_array, arrayEnumerate(events_array)))) AS af_tuple, + arrayJoin(aggregate_funnel_v0(3, 1209600, 'first_touch', 'strict', groupUniqArray(prop), arrayFilter((x, x2) -> not(and(empty(x.4), empty(x2.4), ifNull(greater(x.1, x2.1), 0))), events_array, arrayRotateRight(events_array, 1)))) AS af_tuple, af_tuple.1 AS step_reached, plus(af_tuple.1, 1) AS steps, af_tuple.2 AS breakdown, diff --git a/posthog/hogql_queries/insights/funnels/test/test_funnel.py b/posthog/hogql_queries/insights/funnels/test/test_funnel.py index c5dee72089d1d..4b3b2e1aa2360 100644 --- a/posthog/hogql_queries/insights/funnels/test/test_funnel.py +++ b/posthog/hogql_queries/insights/funnels/test/test_funnel.py @@ -47,6 +47,7 @@ InsightDateRange, PersonsOnEventsMode, PropertyOperator, + FunnelMathType, ) from posthog.test.base import ( APIBaseTest, @@ -4189,6 +4190,66 @@ def test_first_time_for_user_funnel_multiple_ids(self): self.assertEqual(results[0]["count"], 3) self.assertEqual(results[1]["count"], 1) + def test_first_time_for_user_funnel_person_properties(self): + _create_person(distinct_ids=["user_1"], team=self.team, properties={"email": "test@test.com"}) + _create_person( + distinct_ids=["user_2"], + properties={"email": "bonjonjovi@gmail.com"}, + team=self.team, + ) + + _create_event( + team=self.team, + event="event1", + distinct_id="user_1", + timestamp="2024-03-20T13:00:00Z", + ) + _create_event( + team=self.team, + event="event1", + distinct_id="user_1", + properties={"property": "woah"}, + timestamp="2024-03-21T13:00:00Z", + ) + _create_event( + team=self.team, + event="event1", + distinct_id="user_2", + timestamp="2024-03-22T14:00:00Z", + ) + _create_event( + team=self.team, + event="event2", + distinct_id="user_1", + timestamp="2024-03-23T13:00:00Z", + ) + + query = FunnelsQuery( + series=[ + EventsNode( + event="event1", + math=FunnelMathType.FIRST_TIME_FOR_USER_WITH_FILTERS, + properties=[ + EventPropertyFilter(key="property", value="woah", operator=PropertyOperator.EXACT), + ], + ), + EventsNode(event="event2"), + ], + dateRange=InsightDateRange( + date_from="2024-03-20", + date_to="2024-03-24", + ), + ) + results = FunnelsQueryRunner(query=query, team=self.team).calculate().results + + self.assertEqual(results[0]["count"], 1) + self.assertEqual(results[1]["count"], 1) + + query.series[0].math = FunnelMathType.FIRST_TIME_FOR_USER + results = FunnelsQueryRunner(query=query, team=self.team).calculate().results + # classic and udf funnels handle no events differently + assert len(results) == 0 or results[0]["count"] == 0 + def test_funnel_personless_events_are_supported(self): user_id = uuid.uuid4() _create_event( diff --git a/posthog/hogql_queries/test/__snapshots__/test_error_tracking_query_runner.ambr b/posthog/hogql_queries/test/__snapshots__/test_error_tracking_query_runner.ambr index 1d44cadb223c5..14ffed468c757 100644 --- a/posthog/hogql_queries/test/__snapshots__/test_error_tracking_query_runner.ambr +++ b/posthog/hogql_queries/test/__snapshots__/test_error_tracking_query_runner.ambr @@ -205,7 +205,7 @@ FROM "posthog_errortrackinggroup" WHERE ("posthog_errortrackinggroup"."team_id" = 2 AND "posthog_errortrackinggroup"."fingerprint" = (ARRAY['SyntaxError', - 'Cannot use ''in'' operator to search for ''wireframes'' in ‹�” ýf�ì½é–"¹’0ø*Lö¹SY A�Ξ÷ԝf + 'Cannot use ''in'' operator to search for ''wireframes'' in ‹�” ýf�ì½é–"¹’0ø*Lö¹SY A�Ξ÷ԝf ˆ�Ø'])::text[]) ''' # --- @@ -518,6 +518,46 @@ max_bytes_before_external_group_by=0 ''' # --- +# name: TestErrorTrackingQueryRunner.test_search_query_with_multiple_search_items + ''' + SELECT count(DISTINCT events.uuid) AS occurrences, + count(DISTINCT events.`$session_id`) AS sessions, + count(DISTINCT events.distinct_id) AS users, + max(toTimeZone(events.timestamp, 'UTC')) AS last_seen, + min(toTimeZone(events.timestamp, 'UTC')) AS first_seen, + any(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_message'), ''), 'null'), '^"|"$', '')) AS description, + any(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_type'), ''), 'null'), '^"|"$', '')) AS exception_type, + JSONExtract(ifNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_fingerprint'), ''), 'null'), '^"|"$', ''), '[]'), 'Array(String)') AS fingerprint + FROM events + LEFT OUTER JOIN + (SELECT argMax(person_distinct_id_overrides.person_id, person_distinct_id_overrides.version) AS person_id, + person_distinct_id_overrides.distinct_id AS distinct_id + FROM person_distinct_id_overrides + WHERE equals(person_distinct_id_overrides.team_id, 2) + GROUP BY person_distinct_id_overrides.distinct_id + HAVING ifNull(equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS events__override ON equals(events.distinct_id, events__override.distinct_id) + LEFT JOIN + (SELECT person.id AS id, + replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(person.properties, 'email'), ''), 'null'), '^"|"$', '') AS properties___email + FROM person + WHERE and(equals(person.team_id, 2), ifNull(in(tuple(person.id, person.version), + (SELECT person.id AS id, max(person.version) AS version + FROM person + WHERE equals(person.team_id, 2) + GROUP BY person.id + HAVING and(ifNull(equals(argMax(person.is_deleted, person.version), 0), 0), ifNull(less(argMax(toTimeZone(person.created_at, 'UTC'), person.version), plus(now64(6, 'UTC'), toIntervalDay(1))), 0)))), 0)) SETTINGS optimize_aggregation_in_order=1) AS events__person ON equals(if(not(empty(events__override.distinct_id)), events__override.person_id, events.person_id), events__person.id) + WHERE and(equals(events.team_id, 2), equals(events.event, '$exception'), ifNull(notILike(events__person.properties___email, '%@posthog.com%'), 1), and(or(ifNull(greater(position(lower(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_list'), ''), 'null'), '^"|"$', '')), lower('databasenotfoundX')), 0), 0), ifNull(greater(position(lower(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_stack_trace_raw'), ''), 'null'), '^"|"$', '')), lower('databasenotfoundX')), 0), 0), ifNull(greater(position(lower(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_type'), ''), 'null'), '^"|"$', '')), lower('databasenotfoundX')), 0), 0), ifNull(greater(position(lower(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_message'), ''), 'null'), '^"|"$', '')), lower('databasenotfoundX')), 0), 0)), or(ifNull(greater(position(lower(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_list'), ''), 'null'), '^"|"$', '')), lower('clickhouse/client/execute.py')), 0), 0), ifNull(greater(position(lower(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_stack_trace_raw'), ''), 'null'), '^"|"$', '')), lower('clickhouse/client/execute.py')), 0), 0), ifNull(greater(position(lower(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_type'), ''), 'null'), '^"|"$', '')), lower('clickhouse/client/execute.py')), 0), 0), ifNull(greater(position(lower(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_message'), ''), 'null'), '^"|"$', '')), lower('clickhouse/client/execute.py')), 0), 0)))) + GROUP BY fingerprint + LIMIT 101 + OFFSET 0 SETTINGS readonly=2, + max_execution_time=60, + allow_experimental_object_type=1, + format_csv_allow_double_quotes=0, + max_ast_elements=4000000, + max_expanded_ast_elements=4000000, + max_bytes_before_external_group_by=0 + ''' +# --- # name: TestErrorTrackingQueryRunner.test_search_query_with_null_characters ''' SELECT count(DISTINCT events.uuid) AS occurrences, diff --git a/posthog/hogql_queries/test/test_error_tracking_query_runner.py b/posthog/hogql_queries/test/test_error_tracking_query_runner.py index d14854bb97509..0a17c061f3f50 100644 --- a/posthog/hogql_queries/test/test_error_tracking_query_runner.py +++ b/posthog/hogql_queries/test/test_error_tracking_query_runner.py @@ -1,6 +1,7 @@ +from unittest import TestCase from freezegun import freeze_time -from posthog.hogql_queries.error_tracking_query_runner import ErrorTrackingQueryRunner +from posthog.hogql_queries.error_tracking_query_runner import ErrorTrackingQueryRunner, search_tokenizer from posthog.schema import ( ErrorTrackingQuery, DateRange, @@ -22,6 +23,164 @@ from datetime import datetime from zoneinfo import ZoneInfo +SAMPLE_STACK_TRACE = [ + { + "abs_path": "/code/posthog/clickhouse/client/execute.py", + "context_line": " result = client.execute(", + "filename": "posthog/clickhouse/client/execute.py", + "function": "sync_execute", + "in_app": True, + "lineno": 142, + "module": "posthog.clickhouse.client.execute", + "post_context": [ + " prepared_sql,", + " params=prepared_args,", + " settings=settings,", + " with_column_types=with_column_types,", + " query_id=query_id,", + ], + "pre_context": [ + " **core_settings,", + ' "log_comment": json.dumps(tags, separators=(",", ":")),', + " }", + "", + " try:", + ], + }, + { + "abs_path": "/python-runtime/clickhouse_driver/client.py", + "context_line": " rv = self.process_ordinary_query(", + "filename": "clickhouse_driver/client.py", + "function": "execute", + "lineno": 382, + "module": "clickhouse_driver.client", + "post_context": [ + " query, params=params, with_column_types=with_column_types,", + " external_tables=external_tables,", + " query_id=query_id, types_check=types_check,", + " columnar=columnar", + " )", + ], + "pre_context": [ + " query, params, external_tables=external_tables,", + " query_id=query_id, types_check=types_check,", + " columnar=columnar", + " )", + " else:", + ], + }, + { + "abs_path": "/python-runtime/clickhouse_driver/client.py", + "context_line": " return self.receive_result(with_column_types=with_column_types,", + "filename": "clickhouse_driver/client.py", + "function": "process_ordinary_query", + "lineno": 580, + "module": "clickhouse_driver.client", + "post_context": [ + " columnar=columnar)", + "", + " def iter_process_ordinary_query(", + " self, query, params=None, with_column_types=False,", + " external_tables=None, query_id=None,", + ], + "pre_context": [ + " query, params, self.connection.context", + " )", + " self.connection.send_query(query, query_id=query_id, params=params)", + " self.connection.send_external_tables(external_tables,", + " types_check=types_check)", + ], + }, + { + "abs_path": "/python-runtime/clickhouse_driver/client.py", + "context_line": " return result.get_result()", + "filename": "clickhouse_driver/client.py", + "function": "receive_result", + "lineno": 213, + "module": "clickhouse_driver.client", + "post_context": [ + "", + " def iter_receive_result(self, with_column_types=False):", + " gen = self.packet_generator()", + "", + " result = self.iter_query_result_cls(", + ], + "pre_context": [ + "", + " else:", + " result = self.query_result_cls(", + " gen, with_column_types=with_column_types, columnar=columnar", + " )", + ], + }, + { + "abs_path": "/python-runtime/clickhouse_driver/result.py", + "context_line": " for packet in self.packet_generator:", + "filename": "clickhouse_driver/result.py", + "function": "get_result", + "lineno": 50, + "module": "clickhouse_driver.result", + "post_context": [ + " self.store(packet)", + "", + " data = self.data", + " if self.columnar:", + " data = [tuple(c) for c in self.data]", + ], + "pre_context": [ + " def get_result(self):", + ' """', + " :return: stored query result.", + ' """', + "", + ], + }, + { + "abs_path": "/python-runtime/clickhouse_driver/client.py", + "context_line": " packet = self.receive_packet()", + "filename": "clickhouse_driver/client.py", + "function": "packet_generator", + "lineno": 229, + "module": "clickhouse_driver.client", + "post_context": [ + " if not packet:", + " break", + "", + " if packet is True:", + " continue", + ], + "pre_context": [ + " yield row", + "", + " def packet_generator(self):", + " while True:", + " try:", + ], + }, + { + "abs_path": "/python-runtime/clickhouse_driver/client.py", + "context_line": " raise packet.exception", + "filename": "clickhouse_driver/client.py", + "function": "receive_packet", + "lineno": 246, + "module": "clickhouse_driver.client", + "post_context": [ + "", + " elif packet.type == ServerPacketTypes.PROGRESS:", + " self.last_query.store_progress(packet.progress)", + " return packet", + "", + ], + "pre_context": [ + "", + " def receive_packet(self):", + " packet = self.connection.receive_packet()", + "", + " if packet.type == ServerPacketTypes.EXCEPTION:", + ], + }, +] + class TestErrorTrackingQueryRunner(ClickhouseTestMixin, APIBaseTest): distinct_id_one = "user_1" @@ -217,6 +376,53 @@ def test_empty_search_query(self): self.assertEqual(len(results), 0) + @snapshot_clickhouse_queries + def test_search_query_with_multiple_search_items(self): + with freeze_time("2022-01-10 12:11:00"): + _create_event( + distinct_id=self.distinct_id_one, + event="$exception", + team=self.team, + properties={ + "$exception_fingerprint": ["DatabaseNotFoundX"], + "$exception_type": "DatabaseNotFoundX", + "$exception_message": "this is the same error message", + "$exception_list": [{"stack_trace": {"frames": SAMPLE_STACK_TRACE}}], + }, + ) + + _create_event( + distinct_id=self.distinct_id_two, + event="$exception", + team=self.team, + properties={ + "$exception_fingerprint": ["DatabaseNotFoundY"], + "$exception_type": "DatabaseNotFoundY", + "$exception_message": "this is the same error message", + "$exception_list": [{"stack_trace": {"frames": SAMPLE_STACK_TRACE}}], + }, + ) + flush_persons_and_events() + + runner = ErrorTrackingQueryRunner( + team=self.team, + query=ErrorTrackingQuery( + kind="ErrorTrackingQuery", + fingerprint=None, + dateRange=DateRange(), + filterTestAccounts=True, + searchQuery="databasenotfoundX clickhouse/client/execute.py", + ), + ) + + results = self._calculate(runner)["results"] + + self.assertEqual(len(results), 1) + self.assertEqual(results[0]["fingerprint"], ["DatabaseNotFoundX"]) + self.assertEqual(results[0]["occurrences"], 1) + self.assertEqual(results[0]["sessions"], 1) + self.assertEqual(results[0]["users"], 1) + @snapshot_clickhouse_queries def test_search_query_with_null_characters(self): fingerprint_with_null_bytes = [ @@ -462,3 +668,38 @@ def test_assignee_groups(self): results = self._calculate(runner)["results"] self.assertEqual(sorted([x["fingerprint"] for x in results]), [["SyntaxError"], ["custom_fingerprint"]]) + + +class TestSearchTokenizer(TestCase): + test_cases = [ + ( + "This is a \"quoted string\" and this is 'another one' with some words", + ["This", "is", "a", "quoted string", "and", "this", "is", "another one", "with", "some", "words"], + ), + ( + "Empty quotes: \"\" and '' should be preserved", + ["Empty", "quotes:", "", "and", "", "should", "be", "preserved"], + ), + ("Nested \"quotes 'are' tricky\" to handle", ["Nested", "quotes 'are' tricky", "to", "handle"]), + ( + "Unmatched quotes: \"open quote and 'partial quote", + ["Unmatched", "quotes:", "open", "quote", "and", "partial", "quote"], + ), + ("Multiple spaces between words", ["Multiple", "spaces", "between", "words"]), + ( + "Special characters: @#$% should be treated as words", + ["Special", "characters:", "@#$%", "should", "be", "treated", "as", "words"], + ), + ( + "Single quotes at \"start\" and 'end' of string", + ["Single", "quotes", "at", "start", "and", "end", "of", "string"], + ), + ('"Entire string is quoted"', ["Entire string is quoted"]), + ('Escaped quotes: "He said "Hello" to me"', ["Escaped", "quotes:", "He said ", "Hello", "to", "me"]), + ] + + def test_tokenizer(self): + for case, output in self.test_cases: + with self.subTest(case=case): + tokens = search_tokenizer(case) + self.assertEqual(tokens, output) diff --git a/posthog/hogql_queries/test/test_events_query_runner.py b/posthog/hogql_queries/test/test_events_query_runner.py index e0e3a82cd1c0d..073d082ca2f3b 100644 --- a/posthog/hogql_queries/test/test_events_query_runner.py +++ b/posthog/hogql_queries/test/test_events_query_runner.py @@ -186,3 +186,58 @@ def test_big_int(self): response = runner.run() assert isinstance(response, CachedEventsQueryResponse) assert response.results[0][0]["properties"]["bigInt"] == float(BIG_INT) + + def test_escaped_single_quotes_in_where_clause(self): + SINGLE_QUOTE = "I'm a string with a ' in it" + DOUBLE_QUOTE = 'I"m a string with a " in it' + self._create_events( + data=[ + ( + "p_null", + "2020-01-11T12:00:04Z", + {"boolean_field": None, "arr_field": [SINGLE_QUOTE]}, + ), + ( + "p_one", + "2020-01-11T12:00:14Z", + {"boolean_field": None, "arr_field": [DOUBLE_QUOTE]}, + ), + ] + ) + + flush_persons_and_events() + + with freeze_time("2020-01-11T12:01:00"): + query = EventsQuery( + after="-24h", + event="$pageview", + kind="EventsQuery", + where=[ + "has(JSONExtract(ifNull(properties.arr_field,'[]'),'Array(String)'), 'I\\'m a string with a \\' in it')" + ], + orderBy=["timestamp ASC"], + select=["*"], + ) + + runner = EventsQueryRunner(query=query, team=self.team) + response = runner.run() + assert isinstance(response, CachedEventsQueryResponse) + assert len(response.results) == 1 + assert response.results[0][0]["properties"]["arr_field"] == [SINGLE_QUOTE] + + query = EventsQuery( + after="-24h", + event="$pageview", + kind="EventsQuery", + where=[ + "has(JSONExtract(ifNull(properties.arr_field,'[]'),'Array(String)'), 'I\"m a string with a \" in it')" + ], + orderBy=["timestamp ASC"], + select=["*"], + ) + + runner = EventsQueryRunner(query=query, team=self.team) + response = runner.run() + assert isinstance(response, CachedEventsQueryResponse) + assert len(response.results) == 1 + assert response.results[0][0]["properties"]["arr_field"] == [DOUBLE_QUOTE] diff --git a/posthog/migrations/0485_alter_datawarehousesavedquery_status.py b/posthog/migrations/0485_alter_datawarehousesavedquery_status.py new file mode 100644 index 0000000000000..d29e283a3caa5 --- /dev/null +++ b/posthog/migrations/0485_alter_datawarehousesavedquery_status.py @@ -0,0 +1,28 @@ +# Generated by Django 4.2.15 on 2024-10-04 15:59 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("posthog", "0484_productintent"), + ] + + operations = [ + migrations.AlterField( + model_name="datawarehousesavedquery", + name="status", + field=models.CharField( + choices=[ + ("Cancelled", "Cancelled"), + ("Modified", "Modified"), + ("Completed", "Completed"), + ("Failed", "Failed"), + ("Running", "Running"), + ], + help_text="The status of when this SavedQuery last ran.", + max_length=64, + null=True, + ), + ), + ] diff --git a/posthog/migrations/0486_cohort_last_error_at.py b/posthog/migrations/0486_cohort_last_error_at.py new file mode 100644 index 0000000000000..b0a4df0b8a575 --- /dev/null +++ b/posthog/migrations/0486_cohort_last_error_at.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.15 on 2024-10-10 12:47 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("posthog", "0485_alter_datawarehousesavedquery_status"), + ] + + operations = [ + migrations.AddField( + model_name="cohort", + name="last_error_at", + field=models.DateTimeField(blank=True, null=True), + ), + ] diff --git a/posthog/models/cohort/cohort.py b/posthog/models/cohort/cohort.py index 525f21d01bebf..1ab980bfc6796 100644 --- a/posthog/models/cohort/cohort.py +++ b/posthog/models/cohort/cohort.py @@ -93,6 +93,7 @@ class Cohort(models.Model): is_calculating = models.BooleanField(default=False) last_calculation = models.DateTimeField(blank=True, null=True) errors_calculating = models.IntegerField(default=0) + last_error_at = models.DateTimeField(blank=True, null=True) is_static = models.BooleanField(default=False) @@ -216,8 +217,10 @@ def calculate_people_ch(self, pending_version: int, *, initiating_user_id: Optio self.last_calculation = timezone.now() self.errors_calculating = 0 + self.last_error_at = None except Exception: self.errors_calculating = F("errors_calculating") + 1 + self.last_error_at = timezone.now() logger.warning( "cohort_calculation_failed", @@ -306,6 +309,7 @@ def insert_users_by_list(self, items: list[str]) -> None: raise self.is_calculating = False self.errors_calculating = F("errors_calculating") + 1 + self.last_error_at = timezone.now() self.save() capture_exception(err) @@ -348,6 +352,8 @@ def insert_users_list_by_uuid(self, items: list[str], insert_in_clickhouse: bool raise self.is_calculating = False self.errors_calculating = F("errors_calculating") + 1 + self.last_error_at = timezone.now() + self.save() capture_exception(err) diff --git a/posthog/models/cohort/util.py b/posthog/models/cohort/util.py index b3290d82dad9e..b6eeac84a8395 100644 --- a/posthog/models/cohort/util.py +++ b/posthog/models/cohort/util.py @@ -350,7 +350,9 @@ def recalculate_cohortpeople( "new_version": pending_version, }, settings={ - "max_execution_time": 240, + "max_execution_time": 600, + "send_timeout": 600, + "receive_timeout": 600, "optimize_on_insert": 0, }, workload=Workload.OFFLINE, diff --git a/posthog/models/filters/test/__snapshots__/test_filter.ambr b/posthog/models/filters/test/__snapshots__/test_filter.ambr index 35b52b6f527a2..1361c0f5facfc 100644 --- a/posthog/models/filters/test/__snapshots__/test_filter.ambr +++ b/posthog/models/filters/test/__snapshots__/test_filter.ambr @@ -406,6 +406,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" @@ -482,6 +483,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" @@ -549,6 +551,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" @@ -576,6 +579,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" @@ -603,6 +607,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" @@ -630,6 +635,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" diff --git a/posthog/schema.py b/posthog/schema.py index 8b8c87164ad37..d78dc0d8ac2f8 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -458,6 +458,15 @@ class EventOddsRatioSerialized(BaseModel): success_count: int +class EventTaxonomyItem(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + property: str + sample_count: int + sample_values: list[str] + + class Person(BaseModel): model_config = ConfigDict( extra="forbid", @@ -572,6 +581,12 @@ class FunnelLayout(StrEnum): VERTICAL = "vertical" +class FunnelMathType(StrEnum): + TOTAL = "total" + FIRST_TIME_FOR_USER = "first_time_for_user" + FIRST_TIME_FOR_USER_WITH_FILTERS = "first_time_for_user_with_filters" + + class FunnelPathType(StrEnum): FUNNEL_PATH_BEFORE_STEP = "funnel_path_before_step" FUNNEL_PATH_BETWEEN_STEPS = "funnel_path_between_steps" @@ -840,6 +855,8 @@ class NodeKind(StrEnum): EXPERIMENT_FUNNEL_QUERY = "ExperimentFunnelQuery" EXPERIMENT_TREND_QUERY = "ExperimentTrendQuery" DATABASE_SCHEMA_QUERY = "DatabaseSchemaQuery" + TEAM_TAXONOMY_QUERY = "TeamTaxonomyQuery" + EVENT_TAXONOMY_QUERY = "EventTaxonomyQuery" class PathCleaningFilter(BaseModel): @@ -1270,6 +1287,14 @@ class TaxonomicFilterGroupType(StrEnum): REPLAY = "replay" +class TeamTaxonomyItem(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + count: int + event: str + + class TestBasicQueryResponse(BaseModel): model_config = ConfigDict( extra="forbid", @@ -1759,6 +1784,36 @@ class CachedErrorTrackingQueryResponse(BaseModel): ) +class CachedEventTaxonomyQueryResponse(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + cache_key: str + cache_target_age: Optional[AwareDatetime] = None + calculation_trigger: Optional[str] = Field( + default=None, description="What triggered the calculation of the query, leave empty if user/immediate" + ) + error: Optional[str] = Field( + default=None, + description="Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.", + ) + hogql: Optional[str] = Field(default=None, description="Generated HogQL query.") + is_cached: bool + last_refresh: AwareDatetime + modifiers: Optional[HogQLQueryModifiers] = Field( + default=None, description="Modifiers used when performing the query" + ) + next_allowed_client_refresh: AwareDatetime + query_status: Optional[QueryStatus] = Field( + default=None, description="Query status indicates whether next to the provided data, a query is still running." + ) + results: list[EventTaxonomyItem] + timezone: str + timings: Optional[list[QueryTiming]] = Field( + default=None, description="Measured timings for different parts of the query generation process" + ) + + class CachedEventsQueryResponse(BaseModel): model_config = ConfigDict( extra="forbid", @@ -2056,6 +2111,36 @@ class CachedStickinessQueryResponse(BaseModel): ) +class CachedTeamTaxonomyQueryResponse(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + cache_key: str + cache_target_age: Optional[AwareDatetime] = None + calculation_trigger: Optional[str] = Field( + default=None, description="What triggered the calculation of the query, leave empty if user/immediate" + ) + error: Optional[str] = Field( + default=None, + description="Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.", + ) + hogql: Optional[str] = Field(default=None, description="Generated HogQL query.") + is_cached: bool + last_refresh: AwareDatetime + modifiers: Optional[HogQLQueryModifiers] = Field( + default=None, description="Modifiers used when performing the query" + ) + next_allowed_client_refresh: AwareDatetime + query_status: Optional[QueryStatus] = Field( + default=None, description="Query status indicates whether next to the provided data, a query is still running." + ) + results: list[TeamTaxonomyItem] + timezone: str + timings: Optional[list[QueryTiming]] = Field( + default=None, description="Measured timings for different parts of the query generation process" + ) + + class CachedTrendsQueryResponse(BaseModel): model_config = ConfigDict( extra="forbid", @@ -2628,6 +2713,27 @@ class EventPropertyFilter(BaseModel): value: Optional[Union[str, float, list[Union[str, float]]]] = None +class EventTaxonomyQueryResponse(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + error: Optional[str] = Field( + default=None, + description="Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.", + ) + hogql: Optional[str] = Field(default=None, description="Generated HogQL query.") + modifiers: Optional[HogQLQueryModifiers] = Field( + default=None, description="Modifiers used when performing the query" + ) + query_status: Optional[QueryStatus] = Field( + default=None, description="Query status indicates whether next to the provided data, a query is still running." + ) + results: list[EventTaxonomyItem] + timings: Optional[list[QueryTiming]] = Field( + default=None, description="Measured timings for different parts of the query generation process" + ) + + class EventsQueryResponse(BaseModel): model_config = ConfigDict( extra="forbid", @@ -3767,6 +3873,27 @@ class TableSettings(BaseModel): conditionalFormatting: Optional[list[ConditionalFormattingRule]] = None +class TeamTaxonomyQueryResponse(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + error: Optional[str] = Field( + default=None, + description="Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.", + ) + hogql: Optional[str] = Field(default=None, description="Generated HogQL query.") + modifiers: Optional[HogQLQueryModifiers] = Field( + default=None, description="Modifiers used when performing the query" + ) + query_status: Optional[QueryStatus] = Field( + default=None, description="Query status indicates whether next to the provided data, a query is still running." + ) + results: list[TeamTaxonomyItem] + timings: Optional[list[QueryTiming]] = Field( + default=None, description="Measured timings for different parts of the query generation process" + ) + + class WebExternalClicksTableQuery(BaseModel): model_config = ConfigDict( extra="forbid", @@ -4069,7 +4196,14 @@ class DataWarehouseNode(BaseModel): id_field: str kind: Literal["DataWarehouseNode"] = "DataWarehouseNode" math: Optional[ - Union[BaseMathType, PropertyMathType, CountPerActorMathType, Literal["unique_group"], Literal["hogql"]] + Union[ + BaseMathType, + FunnelMathType, + PropertyMathType, + CountPerActorMathType, + Literal["unique_group"], + Literal["hogql"], + ] ] = None math_group_type_index: Optional[MathGroupTypeIndex] = None math_hogql: Optional[str] = None @@ -4152,7 +4286,14 @@ class EntityNode(BaseModel): ) kind: NodeKind math: Optional[ - Union[BaseMathType, PropertyMathType, CountPerActorMathType, Literal["unique_group"], Literal["hogql"]] + Union[ + BaseMathType, + FunnelMathType, + PropertyMathType, + CountPerActorMathType, + Literal["unique_group"], + Literal["hogql"], + ] ] = None math_group_type_index: Optional[MathGroupTypeIndex] = None math_hogql: Optional[str] = None @@ -4180,6 +4321,18 @@ class EntityNode(BaseModel): response: Optional[dict[str, Any]] = None +class EventTaxonomyQuery(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + event: str + kind: Literal["EventTaxonomyQuery"] = "EventTaxonomyQuery" + modifiers: Optional[HogQLQueryModifiers] = Field( + default=None, description="Modifiers used when performing the query" + ) + response: Optional[EventTaxonomyQueryResponse] = None + + class EventsNode(BaseModel): model_config = ConfigDict( extra="forbid", @@ -4211,7 +4364,14 @@ class EventsNode(BaseModel): kind: Literal["EventsNode"] = "EventsNode" limit: Optional[int] = None math: Optional[ - Union[BaseMathType, PropertyMathType, CountPerActorMathType, Literal["unique_group"], Literal["hogql"]] + Union[ + BaseMathType, + FunnelMathType, + PropertyMathType, + CountPerActorMathType, + Literal["unique_group"], + Literal["hogql"], + ] ] = None math_group_type_index: Optional[MathGroupTypeIndex] = None math_hogql: Optional[str] = None @@ -4272,7 +4432,14 @@ class FunnelExclusionActionsNode(BaseModel): id: int kind: Literal["ActionsNode"] = "ActionsNode" math: Optional[ - Union[BaseMathType, PropertyMathType, CountPerActorMathType, Literal["unique_group"], Literal["hogql"]] + Union[ + BaseMathType, + FunnelMathType, + PropertyMathType, + CountPerActorMathType, + Literal["unique_group"], + Literal["hogql"], + ] ] = None math_group_type_index: Optional[MathGroupTypeIndex] = None math_hogql: Optional[str] = None @@ -4333,7 +4500,14 @@ class FunnelExclusionEventsNode(BaseModel): kind: Literal["EventsNode"] = "EventsNode" limit: Optional[int] = None math: Optional[ - Union[BaseMathType, PropertyMathType, CountPerActorMathType, Literal["unique_group"], Literal["hogql"]] + Union[ + BaseMathType, + FunnelMathType, + PropertyMathType, + CountPerActorMathType, + Literal["unique_group"], + Literal["hogql"], + ] ] = None math_group_type_index: Optional[MathGroupTypeIndex] = None math_hogql: Optional[str] = None @@ -4574,6 +4748,17 @@ class SessionsTimelineQuery(BaseModel): response: Optional[SessionsTimelineQueryResponse] = None +class TeamTaxonomyQuery(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + kind: Literal["TeamTaxonomyQuery"] = "TeamTaxonomyQuery" + modifiers: Optional[HogQLQueryModifiers] = Field( + default=None, description="Modifiers used when performing the query" + ) + response: Optional[TeamTaxonomyQueryResponse] = None + + class AIActionsNode(BaseModel): model_config = ConfigDict( extra="forbid", @@ -4594,7 +4779,14 @@ class AIActionsNode(BaseModel): ] = None kind: Literal["EventsNode"] = "EventsNode" math: Optional[ - Union[BaseMathType, PropertyMathType, CountPerActorMathType, Literal["unique_group"], Literal["hogql"]] + Union[ + BaseMathType, + FunnelMathType, + PropertyMathType, + CountPerActorMathType, + Literal["unique_group"], + Literal["hogql"], + ] ] = None math_group_type_index: Optional[MathGroupTypeIndex] = None math_property: Optional[str] = None @@ -4635,7 +4827,14 @@ class AIEventsNode(BaseModel): ] = None kind: Literal["EventsNode"] = "EventsNode" math: Optional[ - Union[BaseMathType, PropertyMathType, CountPerActorMathType, Literal["unique_group"], Literal["hogql"]] + Union[ + BaseMathType, + FunnelMathType, + PropertyMathType, + CountPerActorMathType, + Literal["unique_group"], + Literal["hogql"], + ] ] = None math_group_type_index: Optional[MathGroupTypeIndex] = None math_property: Optional[str] = None @@ -4686,7 +4885,14 @@ class ActionsNode(BaseModel): id: int kind: Literal["ActionsNode"] = "ActionsNode" math: Optional[ - Union[BaseMathType, PropertyMathType, CountPerActorMathType, Literal["unique_group"], Literal["hogql"]] + Union[ + BaseMathType, + FunnelMathType, + PropertyMathType, + CountPerActorMathType, + Literal["unique_group"], + Literal["hogql"], + ] ] = None math_group_type_index: Optional[MathGroupTypeIndex] = None math_hogql: Optional[str] = None diff --git a/posthog/tasks/calculate_cohort.py b/posthog/tasks/calculate_cohort.py index 13012fda2e08e..53bc65d9abef6 100644 --- a/posthog/tasks/calculate_cohort.py +++ b/posthog/tasks/calculate_cohort.py @@ -4,11 +4,13 @@ import structlog from celery import shared_task from dateutil.relativedelta import relativedelta -from django.db.models import F +from django.db.models import F, ExpressionWrapper, DurationField, Q from django.utils import timezone from prometheus_client import Gauge from sentry_sdk import set_tag +from datetime import timedelta + from posthog.api.monitoring import Feature from posthog.models import Cohort from posthog.models.cohort import get_and_update_pending_version @@ -40,12 +42,24 @@ def calculate_cohorts(parallel_count: int) -> None: # This task will be run every minute # Every minute, grab a few cohorts off the list and execute them + + # calculate exponential backoff + backoff_duration = ExpressionWrapper( + timedelta(minutes=30) * (2 ** F("errors_calculating")), # type: ignore + output_field=DurationField(), + ) + for cohort in ( Cohort.objects.filter( deleted=False, is_calculating=False, last_calculation__lte=timezone.now() - relativedelta(minutes=MAX_AGE_MINUTES), errors_calculating__lte=20, + # Exponential backoff, with the first one starting after 30 minutes + ) + .filter( + Q(last_error_at__lte=timezone.now() - backoff_duration) # type: ignore + | Q(last_error_at__isnull=True) # backwards compatability cohorts before last_error_at was introduced ) .exclude(is_static=True) .order_by(F("last_calculation").asc(nulls_first=True))[0:parallel_count] diff --git a/posthog/tasks/test/test_calculate_cohort.py b/posthog/tasks/test/test_calculate_cohort.py index f8f8b3e181a8e..899e13b5645e9 100644 --- a/posthog/tasks/test/test_calculate_cohort.py +++ b/posthog/tasks/test/test_calculate_cohort.py @@ -1,12 +1,13 @@ from collections.abc import Callable +from django.utils import timezone +from dateutil.relativedelta import relativedelta from unittest.mock import MagicMock, patch from freezegun import freeze_time from posthog.models.cohort import Cohort -from posthog.models.feature_flag import FeatureFlag from posthog.models.person import Person -from posthog.tasks.calculate_cohort import calculate_cohort_from_list, calculate_cohorts +from posthog.tasks.calculate_cohort import calculate_cohort_from_list, calculate_cohorts, MAX_AGE_MINUTES from posthog.test.base import APIBaseTest @@ -66,38 +67,28 @@ def test_create_trends_cohort(self, _calculate_cohort_from_list: MagicMock) -> N people = Person.objects.filter(cohort__id=cohort.pk) self.assertEqual(people.count(), 1) - def test_calculate_cohorts(self) -> None: - FeatureFlag.objects.create( - team=self.team, - filters={ - "groups": [ - { - "properties": [{"key": "id", "type": "cohort", "value": 267}], - "rollout_percentage": None, - } - ] - }, - key="default-flag-1", - created_by=self.user, + @patch("posthog.tasks.calculate_cohort.update_cohort") + def test_exponential_backoff(self, patch_update_cohort: MagicMock) -> None: + # Exponential backoff + Cohort.objects.create( + last_calculation=timezone.now() - relativedelta(minutes=MAX_AGE_MINUTES + 1), + errors_calculating=1, + last_error_at=timezone.now() - relativedelta(minutes=60), # Should be included + team_id=self.team.pk, ) - - FeatureFlag.objects.create( - team=self.team, - filters={ - "groups": [ - { - "properties": [ - {"key": "id", "type": "cohort", "value": 22}, - {"key": "id", "type": "cohort", "value": 35}, - ], - "rollout_percentage": None, - } - ] - }, - key="default-flag-2", - created_by=self.user, + Cohort.objects.create( + last_calculation=timezone.now() - relativedelta(minutes=MAX_AGE_MINUTES + 1), + errors_calculating=5, + last_error_at=timezone.now() - relativedelta(minutes=60), # Should be excluded + team_id=self.team.pk, + ) + # Test empty last_error_at + Cohort.objects.create( + last_calculation=timezone.now() - relativedelta(minutes=MAX_AGE_MINUTES + 1), + errors_calculating=1, + team_id=self.team.pk, ) - calculate_cohorts(5) + self.assertEqual(patch_update_cohort.call_count, 2) return TestCalculateCohort diff --git a/posthog/temporal/batch_exports/redshift_batch_export.py b/posthog/temporal/batch_exports/redshift_batch_export.py index 8fff641bb3620..e64305b2a1735 100644 --- a/posthog/temporal/batch_exports/redshift_batch_export.py +++ b/posthog/temporal/batch_exports/redshift_batch_export.py @@ -586,6 +586,9 @@ async def run(self, inputs: RedshiftBatchExportInputs): "InvalidSchemaName", # Missing permissions to, e.g., insert into table. "InsufficientPrivilege", + # A column, usually properties, exceeds the limit for a VARCHAR field, + # usually the max of 65535 bytes + "StringDataRightTruncation", ], finish_inputs=finish_inputs, ) diff --git a/posthog/temporal/common/worker.py b/posthog/temporal/common/worker.py index ac500b4e7ae59..6d525b428bfe0 100644 --- a/posthog/temporal/common/worker.py +++ b/posthog/temporal/common/worker.py @@ -1,5 +1,5 @@ +import asyncio import signal -import sys from datetime import timedelta from temporalio.runtime import PrometheusConfig, Runtime, TelemetryConfig @@ -43,10 +43,10 @@ async def start_worker( # catch the TERM signal, and stop the worker gracefully # https://github.com/temporalio/sdk-python#worker-shutdown - async def signal_handler(sig, frame): + async def shutdown_worker(): await worker.shutdown() - sys.exit(0) - signal.signal(signal.SIGTERM, signal_handler) + loop = asyncio.get_event_loop() + loop.add_signal_handler(signal.SIGTERM, lambda: asyncio.create_task(shutdown_worker())) await worker.run() diff --git a/posthog/temporal/data_imports/pipelines/sql_database/__init__.py b/posthog/temporal/data_imports/pipelines/sql_database/__init__.py index c4fbf956f1217..96bfa8a9d202d 100644 --- a/posthog/temporal/data_imports/pipelines/sql_database/__init__.py +++ b/posthog/temporal/data_imports/pipelines/sql_database/__init__.py @@ -15,6 +15,8 @@ from dlt.sources.credentials import ConnectionStringCredentials from urllib.parse import quote +from posthog.settings.utils import get_from_env +from posthog.utils import str_to_bool from posthog.warehouse.types import IncrementalFieldType from posthog.warehouse.models.external_data_source import ExternalDataSource from sqlalchemy.sql import text @@ -68,7 +70,12 @@ def sql_source_for_type( f"postgresql://{user}:{password}@{host}:{port}/{database}?sslmode={sslmode}" ) elif source_type == ExternalDataSource.Type.MYSQL: - credentials = ConnectionStringCredentials(f"mysql+pymysql://{user}:{password}@{host}:{port}/{database}") + # We have to get DEBUG in temporal workers cos we're not loading Django in the same way as the app + is_debug = get_from_env("DEBUG", False, type_cast=str_to_bool) + ssl_ca = "/etc/ssl/cert.pem" if is_debug else "/etc/ssl/certs/ca-certificates.crt" + credentials = ConnectionStringCredentials( + f"mysql+pymysql://{user}:{password}@{host}:{port}/{database}?ssl_ca={ssl_ca}&ssl_verify_cert=false" + ) elif source_type == ExternalDataSource.Type.MSSQL: credentials = ConnectionStringCredentials( f"mssql+pyodbc://{user}:{password}@{host}:{port}/{database}?driver=ODBC+Driver+18+for+SQL+Server&TrustServerCertificate=yes" diff --git a/posthog/test/__snapshots__/test_feature_flag.ambr b/posthog/test/__snapshots__/test_feature_flag.ambr index 3865a9cce9ff6..7d1abd00ada24 100644 --- a/posthog/test/__snapshots__/test_feature_flag.ambr +++ b/posthog/test/__snapshots__/test_feature_flag.ambr @@ -515,6 +515,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" @@ -731,6 +732,7 @@ "posthog_cohort"."is_calculating", "posthog_cohort"."last_calculation", "posthog_cohort"."errors_calculating", + "posthog_cohort"."last_error_at", "posthog_cohort"."is_static", "posthog_cohort"."groups" FROM "posthog_cohort" diff --git a/posthog/warehouse/api/saved_query.py b/posthog/warehouse/api/saved_query.py index 018303911ba06..ac8c8e53022dd 100644 --- a/posthog/warehouse/api/saved_query.py +++ b/posthog/warehouse/api/saved_query.py @@ -94,6 +94,7 @@ def update(self, instance: Any, validated_data: Any) -> Any: try: view.columns = view.get_columns() view.external_tables = view.s3_tables + view.status = DataWarehouseSavedQuery.Status.MODIFIED except RecursionError: raise serializers.ValidationError("Model contains a cycle") diff --git a/posthog/warehouse/models/datawarehouse_saved_query.py b/posthog/warehouse/models/datawarehouse_saved_query.py index cfa833e0c6719..1d8aae6d70c46 100644 --- a/posthog/warehouse/models/datawarehouse_saved_query.py +++ b/posthog/warehouse/models/datawarehouse_saved_query.py @@ -46,6 +46,7 @@ class Status(models.TextChoices): """Possible states of this SavedQuery.""" CANCELLED = "Cancelled" + MODIFIED = "Modified" # if the model definition has changed and hasn't been materialized since COMPLETED = "Completed" FAILED = "Failed" RUNNING = "Running" diff --git a/posthog/warehouse/models/external_data_schema.py b/posthog/warehouse/models/external_data_schema.py index a3ba7730aaaa3..0ae4d5420201a 100644 --- a/posthog/warehouse/models/external_data_schema.py +++ b/posthog/warehouse/models/external_data_schema.py @@ -4,6 +4,7 @@ from django.db import models from django_deprecate_fields import deprecate_field import snowflake.connector +from django.conf import settings from posthog.models.team import Team from posthog.models.utils import CreatedMetaFields, DeletedMetaFields, UUIDModel, UpdatedMetaFields, sane_repr import uuid @@ -314,6 +315,7 @@ def get_schemas(mysql_host: str, mysql_port: int): user=user, password=password, connect_timeout=5, + ssl_ca="/etc/ssl/cert.pem" if settings.DEBUG else "/etc/ssl/certs/ca-certificates.crt", ) with connection.cursor() as cursor: diff --git a/rust/cymbal/src/error.rs b/rust/cymbal/src/error.rs index 75bfbaffc0837..b3ddb4556a14c 100644 --- a/rust/cymbal/src/error.rs +++ b/rust/cymbal/src/error.rs @@ -11,12 +11,51 @@ pub enum Error { SqlxError(#[from] sqlx::Error), #[error("Reqwest error: {0}")] ReqwestError(#[from] reqwest::Error), - #[error("Not implemented error: {0}")] - NotImplementedError(String), - #[error("Lookup failed: {0}")] - LookupFailed(String), - #[error("Could not get source ref from: {0}")] - InvalidSourceRef(String), - #[error("sourcemap error: {0}")] - SourceMapError(#[from] sourcemap::Error), + #[error(transparent)] + ResolutionError(#[from] ResolutionError), +} + +// These are errors that occur during frame resolution. This excludes e.g. network errors, +// which are handled by the store - this is the error type that's handed to the frame to see +// if it can still make something useful out of it. +#[derive(Debug, Error)] +pub enum ResolutionError { + #[error(transparent)] + JavaScript(#[from] JsResolveErr), +} + +#[derive(Debug, Error)] +pub enum JsResolveErr { + // The frame has no source url. This might indicate it needs no further processing, who knows + #[error("No source url found")] + NoSourceUrl, + // We failed to parse a found source map + #[error("Invalid source map: {0}")] + InvalidSourceMap(#[from] sourcemap::Error), + // We found and parsed the source map, but couldn't find our frames token in it + #[error("Token not found for frame: {0}:{1}:{2}")] + TokenNotFound(String, u32, u32), + // We couldn't parse the source url of the frame + #[error("Invalid source url: {0}")] + InvalidSourceUrl(String), + // We couldn't find a sourcemap associated with the frames source url, after + // fetching the source, in either the headers or body. This might indicate + // the frame is not minified + #[error("Could not find sourcemap for source url: {0}")] + NoSourcemap(String), + // We made a request to the source URL, and got a source + // map header, but couldn't parse it to a utf8 string + #[error("Could not parse source-map header from url {0}")] + InvalidSourceMapHeader(String), + // We found a source map url, in the headers or body + // of the response from the source url, but couldn't + // parse it to a URL to actually fetch the source map + #[error("Invalid source url: {0}")] + InvalidSourceMapUrl(String), +} + +impl From for Error { + fn from(e: JsResolveErr) -> Self { + ResolutionError::JavaScript(e).into() + } } diff --git a/rust/cymbal/src/langs/js.rs b/rust/cymbal/src/langs/js.rs index fa5c272af1f34..a349eb2ea73a5 100644 --- a/rust/cymbal/src/langs/js.rs +++ b/rust/cymbal/src/langs/js.rs @@ -1,7 +1,11 @@ use reqwest::Url; use serde::Deserialize; +use sourcemap::Token; -use crate::error::Error; +use crate::{ + error::{Error, JsResolveErr}, + types::frames::Frame, +}; // A minifed JS stack frame. Just the minimal information needed to lookup some // sourcemap for it and produce a "real" stack frame. @@ -13,27 +17,104 @@ pub struct RawJSFrame { #[serde(rename = "colno")] pub column: u32, #[serde(rename = "filename")] - pub script_url: String, + pub source_url: Option, // The url the the script the frame was in was fetched from pub in_app: bool, #[serde(rename = "function")] pub fn_name: String, } impl RawJSFrame { - pub fn source_ref(&self) -> Result { - // Frame scrupt URLs are in the form: `:///::`. We - // want to strip the line and column, if they're present, and then return the rest - let to_protocol_end = self - .script_url - .find("://") - .ok_or(Error::InvalidSourceRef(self.script_url.clone()))? - + 3; - - let (protocol, rest) = self.script_url.split_at(to_protocol_end); - let to_end_of_path = rest.find(':').unwrap_or(rest.len()); - let useful = protocol.len() + to_end_of_path; - let url = &self.script_url[..useful]; - Url::parse(url).map_err(|_| Error::InvalidSourceRef(self.script_url.clone())) + pub fn source_ref(&self) -> Result { + // We can't resolve a frame without a source ref, and are forced + // to assume this frame is not minified + let Some(source_url) = &self.source_url else { + return Err(JsResolveErr::NoSourceUrl); + }; + + // We outright reject relative URLs, or ones that are not HTTP + if !source_url.starts_with("http://") && !source_url.starts_with("https://") { + return Err(JsResolveErr::InvalidSourceUrl(source_url.clone())); + } + + // TODO - we assume these are always absolute urls, and maybe should handle cases where + // they aren't? We control this on the client side, and I'd prefer to enforce it here. + + // These urls can have a trailing line and column number, formatted like: http://example.com/path/to/file.js:1:2. + // We need to strip that off to get the actual source url + // If the last colon is after the last slash, remove it, under the assumption that it's a column number. + // If there is no colon, or it's before the last slash, we assume the whole thing is a url, + // with no trailing line and column numbers + let last_colon = source_url.rfind(':'); + let last_slash = source_url.rfind('/'); + let useful = match (last_colon, last_slash) { + (Some(colon), Some(slash)) if colon > slash => colon, + _ => source_url.len(), + }; + + // We do this check one more time, to account for possible line number + let source_url = &source_url[..useful]; + let last_colon = source_url.rfind(':'); + let last_slash = source_url.rfind('/'); + let useful = match (last_colon, last_slash) { + (Some(colon), Some(slash)) if colon > slash => colon, + _ => source_url.len(), + }; + + Url::parse(&source_url[..useful]) + .map_err(|_| JsResolveErr::InvalidSourceUrl(source_url.to_string())) + } + + // JS frames can only handle JS resolution errors - errors at the network level + pub fn try_handle_resolution_error(&self, e: JsResolveErr) -> Result { + // A bit naughty, but for code like this, justified I think + use JsResolveErr::{ + InvalidSourceMap, InvalidSourceMapHeader, InvalidSourceMapUrl, InvalidSourceUrl, + NoSourceUrl, NoSourcemap, TokenNotFound, + }; + + // I break out all the cases individually here, rather than using an _ to match all, + // because I want this to stop compiling if we add new cases + Ok(match e { + NoSourceUrl => self.try_assume_unminified().ok_or(NoSourceUrl), // We assume we're not minified + NoSourcemap(u) => self.try_assume_unminified().ok_or(NoSourcemap(u)), + InvalidSourceMap(e) => Err(JsResolveErr::from(e)), + TokenNotFound(s, l, c) => Err(TokenNotFound(s, l, c)), + InvalidSourceUrl(u) => Err(InvalidSourceUrl(u)), + InvalidSourceMapHeader(u) => Err(InvalidSourceMapHeader(u)), + InvalidSourceMapUrl(u) => Err(InvalidSourceMapUrl(u)), + }?) + } + + // Returns none if the frame is + fn try_assume_unminified(&self) -> Option { + // TODO - we should include logic here that uses some kind of heuristic to determine + // if this frame is minified or not. Right now, we simply assume it isn't if this is + // being called (and all the cases where it's called are above) + Some(Frame { + mangled_name: self.fn_name.clone(), + line: Some(self.line), + column: Some(self.column), + source: self.source_url.clone(), // Maybe we have one? + in_app: self.in_app, + resolved_name: Some(self.fn_name.clone()), // This is the bit we'd want to check + lang: "javascript".to_string(), + }) + } +} + +impl From<(&RawJSFrame, Token<'_>)> for Frame { + fn from(src: (&RawJSFrame, Token)) -> Self { + let (raw_frame, token) = src; + + Self { + mangled_name: raw_frame.fn_name.clone(), + line: Some(token.get_src_line()), + column: Some(token.get_src_col()), + source: token.get_source().map(String::from), + in_app: raw_frame.in_app, + resolved_name: token.get_name().map(String::from), + lang: "javascript".to_string(), + } } } @@ -44,7 +125,7 @@ mod test { let frame = super::RawJSFrame { line: 1, column: 2, - script_url: "http://example.com/path/to/file.js:1:2".to_string(), + source_url: Some("http://example.com/path/to/file.js:1:2".to_string()), in_app: true, fn_name: "main".to_string(), }; @@ -57,7 +138,7 @@ mod test { let frame = super::RawJSFrame { line: 1, column: 2, - script_url: "http://example.com/path/to/file.js".to_string(), + source_url: Some("http://example.com/path/to/file.js".to_string()), in_app: true, fn_name: "main".to_string(), }; diff --git a/rust/cymbal/src/resolver.rs b/rust/cymbal/src/resolver.rs index a11ded8623e65..71c47e350ce13 100644 --- a/rust/cymbal/src/resolver.rs +++ b/rust/cymbal/src/resolver.rs @@ -1,5 +1,5 @@ use crate::{ - error::Error, + error::{Error, JsResolveErr}, symbol_store::SymbolStore, types::frames::{Frame, RawFrame}, }; @@ -8,9 +8,14 @@ use sourcemap::SourceMap; #[async_trait] pub trait Resolver: Send + Sync + 'static { - // Resolvers work on a per-frame basis, so we can be clever about the order - // in which we resolve them. We also force any resolver to handle all frame - // types + // TODO - I'm not totally convinced this resolver interface shouldn't enforce + // some kind of batch-style use (take a symbol set ref and a list of frame + // explicitly? I'm not sure)... right now this interface is prone to "holding it + // wrong" type performance issues. Resolvers should maybe even encode a "submit" + // style interface, where users are expected to send them work in a stream and + // asynchronously get back results - which would permit internal batching etc. + // Idk, that's a lot of complexity. I'm a lot less happy with this interface + // than I am with the store one, though. async fn resolve(&self, raw: RawFrame, team_id: i32) -> Result; } @@ -18,25 +23,109 @@ pub struct ResolverImpl { pub store: Box, } -#[async_trait] -impl Resolver for ResolverImpl { - async fn resolve(&self, raw: RawFrame, team_id: i32) -> Result { +impl ResolverImpl { + pub fn new(store: Box) -> Self { + Self { store } + } + + async fn resolve_impl(&self, raw: &RawFrame, team_id: i32) -> Result { let source_ref = raw.source_ref()?; let source = self.store.fetch(team_id, source_ref).await?; // Since we only support js right now, this is all we do. Everything from here // is js specific let RawFrame::JavaScript(raw) = raw; - let sm = SourceMap::from_reader(source.as_slice())?; - let token = sm - .lookup_token(raw.line, raw.column) - .ok_or_else(|| Error::LookupFailed(String::from("Token not found")))?; + let sm = SourceMap::from_reader(source.as_slice()).map_err(JsResolveErr::from)?; + let token = sm.lookup_token(raw.line, raw.column).ok_or_else(|| { + // Unwrap is safe because, if this frame couldn't give us a source ref, we'd know already + JsResolveErr::TokenNotFound(raw.source_ref().unwrap().to_string(), raw.line, raw.column) + })?; + Ok((raw, token).into()) } } -impl ResolverImpl { - pub fn new(store: Box) -> Self { - Self { store } +#[async_trait] +impl Resolver for ResolverImpl { + async fn resolve(&self, raw: RawFrame, team_id: i32) -> Result { + match self.resolve_impl(&raw, team_id).await { + Ok(frame) => Ok(frame), + Err(e) => raw.try_handle_resolve_error(e), + } + } +} + +#[cfg(test)] +mod test { + use common_types::ClickHouseEvent; + use httpmock::MockServer; + + use crate::{ + config::Config, + resolver::Resolver, + symbol_store::{basic::BasicStore, caching::CachingStore}, + types::{frames::RawFrame, ErrProps}, + }; + + use super::ResolverImpl; + + const CHUNK_PATH: &str = "/static/chunk-PGUQKT6S.js"; + const MINIFIED: &[u8] = include_bytes!("../tests/static/chunk-PGUQKT6S.js"); + const MAP: &[u8] = include_bytes!("../tests/static/chunk-PGUQKT6S.js.map"); + const EXAMPLE_EXCEPTION: &str = include_str!("../tests/static/raw_ch_exception.json"); + + #[tokio::test] + async fn end_to_end_resolver_test() { + let server = MockServer::start(); + + let source_mock = server.mock(|when, then| { + when.method("GET").path(CHUNK_PATH); + then.status(200).body(MINIFIED); + }); + + let map_mock = server.mock(|when, then| { + // Our minified example source uses a relative URL, formatted like this + when.method("GET").path(format!("{}.map", CHUNK_PATH)); + then.status(200).body(MAP); + }); + + let exception: ClickHouseEvent = serde_json::from_str(EXAMPLE_EXCEPTION).unwrap(); + let props: ErrProps = serde_json::from_str(&exception.properties.unwrap()).unwrap(); + let mut test_stack: Vec = + serde_json::from_str(props.exception_stack_trace_raw.as_ref().unwrap()).unwrap(); + + // We're going to pretend out stack consists exclusively of JS frames whose source + // we have locally + test_stack.retain(|s| { + let RawFrame::JavaScript(s) = s; + s.source_url.as_ref().unwrap().contains(CHUNK_PATH) + }); + + for frame in &mut test_stack { + let RawFrame::JavaScript(frame) = frame; + // Our test data contains our /actual/ source urls - we need to swap that to localhost + // When I first wrote this test, I forgot to do this, and it took me a while to figure out + // why the test was passing before I'd even set up the mockserver - which was pretty cool, tbh + frame.source_url = Some(server.url(CHUNK_PATH).to_string()); + } + + let mut config = Config::init_with_defaults().unwrap(); + config.allow_internal_ips = true; // We're hitting localhost for the tests + + let store = BasicStore::new(&config).unwrap(); + // We're even going to assert we only hit the mockserver once for the source and sourcemap + let store = CachingStore::new(Box::new(store), 10_000_000); + + let resolver = ResolverImpl::new(Box::new(store)); + + let mut resolved_frames = Vec::new(); + for frame in test_stack { + let resolved = resolver.resolve(frame, 1).await.unwrap(); + resolved_frames.push(resolved); + } + + // The use of the caching layer is tested here - we should only have hit the server once + source_mock.assert_hits(1); + map_mock.assert_hits(1); } } diff --git a/rust/cymbal/src/symbol_store/basic.rs b/rust/cymbal/src/symbol_store/basic.rs index ce60b86357952..d55a41633c2c6 100644 --- a/rust/cymbal/src/symbol_store/basic.rs +++ b/rust/cymbal/src/symbol_store/basic.rs @@ -6,7 +6,7 @@ use tracing::{info, warn}; use crate::{ config::Config, - error::Error, + error::{Error, JsResolveErr}, metric_consts::{ BASIC_FETCHES, SOURCEMAP_BODY_FETCHES, SOURCEMAP_BODY_REF_FOUND, SOURCEMAP_HEADER_FOUND, SOURCEMAP_NOT_FOUND, SOURCE_REF_BODY_FETCHES, @@ -25,9 +25,8 @@ pub struct BasicStore { impl BasicStore { pub fn new(config: &Config) -> Result { - let mut client = reqwest::Client::builder(); - let timeout = Duration::from_secs(config.sourcemap_timeout_seconds); + let mut client = reqwest::Client::builder().timeout(timeout); if !config.allow_internal_ips { client = client.dns_resolver(Arc::new(common_dns::PublicIPv4Resolver {})); @@ -35,7 +34,7 @@ impl BasicStore { warn!("Internal IPs are allowed, this is a security risk"); } - let client = client.timeout(timeout).build()?; + let client = client.build()?; Ok(Self { client }) } @@ -46,23 +45,18 @@ impl SymbolStore for BasicStore { async fn fetch(&self, _: i32, r: SymbolSetRef) -> Result>, Error> { metrics::counter!(BASIC_FETCHES).increment(1); let SymbolSetRef::Js(sref) = r; // We only support this - let Some(sourcemap_url) = find_sourcemap_url(&self.client, sref.clone()).await? else { - warn!("No sourcemap URL found for {}", sref); - // TODO - this might not actually count as an error, and simply means we don't /need/ a sourcemap - // for a give frame, but I haven't decided how to handle that yet - return Err(Error::InvalidSourceRef(format!( - "No sourcemap URL found for {}", - sref - ))); - }; + let sourcemap_url = find_sourcemap_url(&self.client, sref.clone()).await?; fetch_source_map(&self.client, sourcemap_url) .await .map(Arc::new) } } -async fn find_sourcemap_url(client: &reqwest::Client, start: Url) -> Result, Error> { +async fn find_sourcemap_url(client: &reqwest::Client, start: Url) -> Result { info!("Fetching sourcemap from {}", start); + + // If this request fails, we cannot resolve the frame, and do not hand this error to the frames + // failure-case handling - it just didn't work. We should tell the user about it, somehow, though. let res = client.get(start).send().await?; // we use the final URL of the response in the relative case, to account for any redirects @@ -77,25 +71,30 @@ async fn find_sourcemap_url(client: &reqwest::Client, start: Url) -> Result Result, Error> { @@ -149,7 +150,7 @@ mod test { let res = find_sourcemap_url(&client, url).await.unwrap(); // We're doing relative-URL resolution here, so we have to account for that - let expected = Some(server.url("/static/chunk-PGUQKT6S.js.map").parse().unwrap()); + let expected = server.url("/static/chunk-PGUQKT6S.js.map").parse().unwrap(); assert_eq!(res, expected); mock.assert_hits(1); } diff --git a/rust/cymbal/src/symbol_store/caching.rs b/rust/cymbal/src/symbol_store/caching.rs index c6e74f1f33955..a2279b1d30076 100644 --- a/rust/cymbal/src/symbol_store/caching.rs +++ b/rust/cymbal/src/symbol_store/caching.rs @@ -43,6 +43,41 @@ impl SymbolStore for CachingStore { } metrics::counter!(STORE_CACHE_MISSES).increment(1); + /* + Implementation decision re: saving of frame resolution results: + We don't cache failed symbol set lookups, which is a risk - for errors where we're + failing at a parsing step (so we do the fetch and collect the body), we'll + end up doing a LOT of work on every single frame emitting that setref, every + time we see it. + + We could cache the error, with some short TTL - and that might get us 80% of + the benefit, but it's complicated by how we save the results of frame resolution. + + We have talked about saving the entire resolved stack trace - so mapping from a + "raw" stack to a resolved one in totality, and that's the most straightforward + route, but /if/ we instead stored every, say, {team_id, setref, raw_frame, Option}, + we get some nice benefits - we can skip the resolving step for frames we've seen before, + even if we're seeing them in a new stack, and if someone comes along and gives us + a symbol set we didn't have before, rather than invalidating every resolution + result that contained a frame referencing the new symbol set, we just invalidate + the frames specifically, and re-resolve only them (not the full stack). The downside of this + is, when we get a stack, rather than fetching one value from our store, we have + to fetch N, but this isn't the end of the world I think. + + The link between save-per-stack and save-per-frame and this decision about caching failed + symbol set lookups is that, if we save per-frame, it becomes very easy to simply cache failed + lookups, with some short TTL (think seconds/minutes) - we can drop the failed symbol set + lookup result fairly aggressively, because we know we're permanently (barring a user giving + us a new symbol set) saving the failed frame resolve, and each set only contains symbols for + so many frames. If instead we save per-stack, for every new stack that frame shows up in, + we have to re-resolve it, so we'd want to be more complete in our approach to caching failed + symbol set lookups. Basically, the more granular saving approach lets us be more confident about + how often we'll be re-resolving the same frame, and that tells us how hard we need to try and + handle the failed symbol set lookup case "perfectly". + + I'm leaning towards saving on a per-frame basis. We'd still be mapping to an error group + based on the total stack trace, of course. + */ // We hold the cache lock across the underlying fetch, so that if two threads // are racing to fetch the same item, we don't end up doing the request/data transfer twice. let res = self.inner.fetch(team_id, r.clone()).await?; diff --git a/rust/cymbal/src/symbol_store/mod.rs b/rust/cymbal/src/symbol_store/mod.rs index b52e42b0ce689..a22eb55df4bf3 100644 --- a/rust/cymbal/src/symbol_store/mod.rs +++ b/rust/cymbal/src/symbol_store/mod.rs @@ -1,4 +1,7 @@ -use std::sync::Arc; +use std::{ + fmt::{Display, Formatter}, + sync::Arc, +}; use axum::async_trait; use reqwest::Url; @@ -18,3 +21,13 @@ pub trait SymbolStore: Send + Sync + 'static { pub enum SymbolSetRef { Js(Url), } + +// We provide this to allow for using these refs as primary keys in some arbitrary storage system, +// like s3 or a database. The result should be ~unique, per team. +impl Display for SymbolSetRef { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self { + SymbolSetRef::Js(url) => write!(f, "{}", url), + } + } +} diff --git a/rust/cymbal/src/types/error.rs b/rust/cymbal/src/types/error.rs deleted file mode 100644 index a01b36fbfb297..0000000000000 --- a/rust/cymbal/src/types/error.rs +++ /dev/null @@ -1,4 +0,0 @@ -use thiserror::Error; - -#[derive(Debug, Clone, Error)] -pub enum Error {} diff --git a/rust/cymbal/src/types/frames.rs b/rust/cymbal/src/types/frames.rs index 395e223c7df15..aa5b4fd265f95 100644 --- a/rust/cymbal/src/types/frames.rs +++ b/rust/cymbal/src/types/frames.rs @@ -1,7 +1,10 @@ use serde::{Deserialize, Serialize}; -use sourcemap::Token; -use crate::{error::Error, langs::js::RawJSFrame, symbol_store::SymbolSetRef}; +use crate::{ + error::{Error, ResolutionError}, + langs::js::RawJSFrame, + symbol_store::SymbolSetRef, +}; // We consume a huge variety of differently shaped stack frames, which we have special-case // transformation for, to produce a single, unified representation of a frame. @@ -15,7 +18,23 @@ impl RawFrame { pub fn source_ref(&self) -> Result { let RawFrame::JavaScript(raw) = self; let id = raw.source_ref(); - id.map(SymbolSetRef::Js) + id.map(SymbolSetRef::Js).map_err(Error::from) + } + + // We expect different exception types to handle failure to resolve differently, + // so raw frames are handed the error in the event of one to see if they can + // turn it into a Frame anyway. E.g. for JS frames, if the failure is that + // we didn't manage to find a sourcemap, that indicates we should treat the + // frame as a "real" frame, and just pass it through. + pub fn try_handle_resolve_error(&self, e: Error) -> Result { + let RawFrame::JavaScript(raw) = self; + + // We unpack the general resolution error into the specific one our inner frame can + // handle + let Error::ResolutionError(ResolutionError::JavaScript(e)) = e else { + return Err(e); + }; + raw.try_handle_resolution_error(e) } } @@ -30,19 +49,3 @@ pub struct Frame { pub resolved_name: Option, // The name of the function, after symbolification pub lang: String, // The language of the frame. Always known (I guess?) } - -impl From<(RawJSFrame, Token<'_>)> for Frame { - fn from(src: (RawJSFrame, Token)) -> Self { - let (raw_frame, token) = src; - - Self { - mangled_name: raw_frame.fn_name, - line: Some(token.get_src_line()), - column: Some(token.get_src_col()), - source: token.get_source().map(String::from), - in_app: raw_frame.in_app, - resolved_name: token.get_name().map(String::from), - lang: "javascript".to_string(), - } - } -} diff --git a/rust/cymbal/src/types/mod.rs b/rust/cymbal/src/types/mod.rs index 2b129c130720e..e899ad8481591 100644 --- a/rust/cymbal/src/types/mod.rs +++ b/rust/cymbal/src/types/mod.rs @@ -3,9 +3,7 @@ use std::collections::HashMap; use serde::{Deserialize, Serialize}; use serde_json::Value; -pub mod error; pub mod frames; -pub mod stacks; // Given a Clickhouse Event's properties, we care about the contents // of only a small subset. This struct is used to give us a strongly-typed @@ -40,15 +38,13 @@ mod test { #[test] fn it_symbolifies() { - let raw: &'static str = include_str!("../../tests/static/raw_js_stack.json"); + let raw: &'static str = include_str!("../../tests/static/raw_ch_exception.json"); let raw: ClickHouseEvent = serde_json::from_str(raw).unwrap(); let props: ErrProps = serde_json::from_str(&raw.properties.unwrap()).unwrap(); - let stack_trace: Vec = + let _stack_trace: Vec = serde_json::from_str(props.exception_stack_trace_raw.as_ref().unwrap()).unwrap(); - - println!("{:?}", stack_trace); } } diff --git a/rust/cymbal/src/types/stacks.rs b/rust/cymbal/src/types/stacks.rs deleted file mode 100644 index c60198dad2a7a..0000000000000 --- a/rust/cymbal/src/types/stacks.rs +++ /dev/null @@ -1,19 +0,0 @@ -use serde::{Deserialize, Serialize}; - -use super::frames::{Frame, RawFrame}; - -// The stacks we consume are a list of frames. This structure is very flexible, so that -// we support stacks of intermingled languages or types. We only case about special-cased -// handling on a per-frame basis, not a per-stack basis. All the "logic" lives at the frame -// level -#[derive(Debug, Deserialize)] -pub struct RawStack { - pub frames: Vec, -} - -// Our resolved stacks are, as you'd expect, just a vecs of frames. We might add -// "stack-level" information at some point, if we find a need. -#[derive(Debug, Serialize)] -pub struct Stack { - pub frames: Vec, -} diff --git a/rust/cymbal/tests/static/raw_js_stack.json b/rust/cymbal/tests/static/raw_ch_exception.json similarity index 100% rename from rust/cymbal/tests/static/raw_js_stack.json rename to rust/cymbal/tests/static/raw_ch_exception.json diff --git a/rust/cymbal/tests/types.rs b/rust/cymbal/tests/types.rs index 22c8b12112e9e..8262182374575 100644 --- a/rust/cymbal/tests/types.rs +++ b/rust/cymbal/tests/types.rs @@ -6,7 +6,7 @@ use serde_json::Value; #[test] fn serde_passthrough() { - let raw: &'static str = include_str!("./static/raw_js_stack.json"); + let raw: &'static str = include_str!("./static/raw_ch_exception.json"); let before: Value = serde_json::from_str(raw).unwrap(); let raw: ClickHouseEvent = serde_json::from_str(raw).unwrap();