Skip to content

Commit

Permalink
feat(grouping): Add project option to force grouping and persist it (#…
Browse files Browse the repository at this point in the history
…12432)

* feat(grouping): Add project option to force grouping and persit it

* feat(grouping): Expose the grouping config forcing through the api

* ref: Improved grouping config selectiong

* feat(grouping): Add grouping config to the UI (feature flagged)

* meta: Bump semaphore to 0.4.20

* ref: Move grouping config freeze into normalize and change apis

* ref: Cleaned up config code

* ref: Add config ID to the grouping info output

* fix(api): Fix bug where parser didn't allow dates ending in `Z`, and didn't correctly report the parse fail to the user (ISSUE-376)

Our parser didn't allow for dates with `Z` at the end, so updated the regex to allow. Also fixed a
bug where if a date started with a valid date but ended with invalid input then we'd create a search
filter from the valid input, then shove the invalid input into `message` rather than raising a parse
error.

Fixes (ISSUE-376)

* Add enhanced privacy to feedback (#12418)

* feat(app-platform): Issue Link UI (#12345)

This change takes care of dynamically rendering the Link and Create
forms for Sentry Apps that support Issue Link components.

* add project to payload (#12407)

* feat: Added grouping selector on grouping info page

* feat: Added tooltip

* test: Improve test coverage for grouping

* ref: Slightly improved native grouping

* ref: More cleanup in new grouping algorithm

* ref: rename a misnamed function
  • Loading branch information
mitsuhiko authored Mar 20, 2019
1 parent befa597 commit 1fcc598
Show file tree
Hide file tree
Showing 33 changed files with 550 additions and 267 deletions.
2 changes: 1 addition & 1 deletion requirements-base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ redis>=2.10.3,<2.10.6
requests-oauthlib==0.3.3
requests[security]>=2.20.0,<2.21.0
selenium==3.141.0
semaphore>=0.4.19,<0.5.0
semaphore>=0.4.20,<0.5.0
sentry-sdk>=0.7.0
setproctitle>=1.1.7,<1.2.0
simplejson>=3.2.0,<3.9.0
Expand Down
13 changes: 12 additions & 1 deletion src/sentry/api/endpoints/event_grouping_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from sentry.api.base import Endpoint
from sentry.api.bases.group import GroupPermission
from sentry.api.exceptions import ResourceDoesNotExist
from sentry.grouping.api import ConfigNotFoundException
from sentry.models import Event
from sentry.utils import json

Expand All @@ -31,9 +32,19 @@ def get(self, request, event_id):
Event.objects.bind_nodes([event], 'data')

rv = {}
config_name = request.GET.get('config') or None

# We always fetch the stored hashes here. The reason for this is
# that we want to show in the UI if the forced grouping algorithm
# produced hashes that would normally also appear in the event.
hashes = event.get_hashes()

for (key, variant) in six.iteritems(event.get_grouping_variants()):
try:
variants = event.get_grouping_variants(config_name)
except ConfigNotFoundException:
raise ResourceDoesNotExist(detail='Unknown grouping config')

for (key, variant) in six.iteritems(variants):
d = variant.as_dict()
# Since the hashes are generated on the fly and might no
# longer match the stored ones we indicate if the hash
Expand Down
17 changes: 17 additions & 0 deletions src/sentry/api/endpoints/grouping_configs.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
from __future__ import absolute_import

from rest_framework.response import Response

from sentry.api.base import Endpoint
from sentry.api.serializers import serialize
from sentry.grouping.strategies.configurations import CONFIGURATIONS


class GroupingConfigsEndpoint(Endpoint):
permission_classes = ()

def get(self, request):
return Response(serialize([
config().as_dict() for config in sorted(CONFIGURATIONS.values(),
key=lambda x: x.id)
]))
9 changes: 9 additions & 0 deletions src/sentry/api/endpoints/project_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ class ProjectAdminSerializer(ProjectMemberSerializer):
storeCrashReports = serializers.BooleanField(required=False)
relayPiiConfig = serializers.CharField(required=False)
scrubIPAddresses = serializers.BooleanField(required=False)
groupingConfig = serializers.CharField(required=False)
scrapeJavaScript = serializers.BooleanField(required=False)
allowedDomains = ListField(child=OriginField(), required=False)
resolveAge = serializers.IntegerField(required=False)
Expand Down Expand Up @@ -347,6 +348,9 @@ def put(self, request, project):
if result.get('scrubIPAddresses') is not None:
if project.update_option('sentry:scrub_ip_address', result['scrubIPAddresses']):
changed_proj_settings['sentry:scrub_ip_address'] = result['scrubIPAddresses']
if result.get('groupingConfig') is not None:
if project.update_option('sentry:grouping_config', result['groupingConfig']):
changed_proj_settings['sentry:grouping_config'] = result['groupingConfig']
if result.get('securityToken') is not None:
if project.update_option('sentry:token', result['securityToken']):
changed_proj_settings['sentry:token'] = result['securityToken']
Expand Down Expand Up @@ -444,6 +448,11 @@ def put(self, request, project):
'sentry:scrub_ip_address',
bool(options['sentry:scrub_ip_address']),
)
if 'sentry:grouping_config' in options:
project.update_option(
'sentry:grouping_config',
options['sentry:grouping_config'],
)
if 'mail:subject_prefix' in options:
project.update_option(
'mail:subject_prefix',
Expand Down
1 change: 1 addition & 0 deletions src/sentry/api/serializers/models/event.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ def serialize(self, obj, attrs, user):
'dateReceived': received,
'errors': errors,
'fingerprints': obj.get_hashes(),
'groupingConfig': obj.get_grouping_config(),
'_meta': {
'entries': attrs['_meta']['entries'],
'message': message_meta,
Expand Down
3 changes: 3 additions & 0 deletions src/sentry/api/serializers/models/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
EnvironmentProject, Project, ProjectAvatar, ProjectBookmark, ProjectOption, ProjectPlatform,
ProjectStatus, ProjectTeam, Release, ReleaseProjectEnvironment, Deploy, UserOption, DEFAULT_SUBJECT_TEMPLATE
)
from sentry.grouping.strategies.configurations import DEFAULT_CONFIG as DEFAULT_GROUPING_CONFIG
from sentry.utils.data_filters import FilterTypes
from sentry.utils.db import is_postgres

Expand Down Expand Up @@ -423,6 +424,7 @@ class DetailedProjectSerializer(ProjectWithTeamSerializer):
'sentry:token_header',
'sentry:verify_ssl',
'sentry:scrub_ip_address',
'sentry:grouping_config',
'sentry:relay_pii_config',
'feedback:branding',
'digests:mail:minimum_delay',
Expand Down Expand Up @@ -540,6 +542,7 @@ def serialize(self, obj, attrs, user):
'verifySSL': bool(attrs['options'].get('sentry:verify_ssl', False)),
'scrubIPAddresses': bool(attrs['options'].get('sentry:scrub_ip_address', False)),
'scrapeJavaScript': bool(attrs['options'].get('sentry:scrape_javascript', True)),
'groupingConfig': attrs['options'].get('sentry:grouping_config') or DEFAULT_GROUPING_CONFIG,
'organization':
attrs['org'],
'plugins':
Expand Down
7 changes: 7 additions & 0 deletions src/sentry/api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@
from .endpoints.user_subscriptions import UserSubscriptionsEndpoint
from .endpoints.event_file_committers import EventFileCommittersEndpoint
from .endpoints.setup_wizard import SetupWizard
from .endpoints.grouping_configs import GroupingConfigsEndpoint


urlpatterns = patterns(
Expand Down Expand Up @@ -1262,6 +1263,12 @@
name='sentry-api-0-sentry-app-authorizations'
),

# Grouping configs
url(
r'^grouping-configs/$', GroupingConfigsEndpoint.as_view(),
name='sentry-api-0-grouping-configs'
),

# Internal
url(r'^internal/health/$', SystemHealthEndpoint.as_view(),
name='sentry-api-0-system-health'),
Expand Down
2 changes: 2 additions & 0 deletions src/sentry/conf/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,8 @@ def create_partitioned_queues(name):
'organizations:global-views': False,
# Turns on grouping info.
'organizations:grouping-info': False,
# Lets organizations manage grouping configs
'organizations:set-grouping-config': False,
# Enable integration functionality to create and link groups to issues on
# external services.
'organizations:integrations-issue-basic': False,
Expand Down
16 changes: 12 additions & 4 deletions src/sentry/event_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from sentry.constants import (
LOG_LEVELS, LOG_LEVELS_MAP, VALID_PLATFORMS, MAX_TAG_VALUE_LENGTH,
)
from sentry.grouping.api import get_grouping_config_dict_for_project
from sentry.coreapi import (
APIError,
APIForbidden,
Expand Down Expand Up @@ -338,6 +339,7 @@ def __init__(
data,
version='5',
project=None,
grouping_config=None,
client_ip=None,
user_agent=None,
auth=None,
Expand All @@ -348,6 +350,9 @@ def __init__(
self._data = _decode_event(data, content_encoding=content_encoding)
self.version = version
self._project = project
if grouping_config is None and project is not None:
grouping_config = get_grouping_config_dict_for_project(self._project)
self._grouping_config = grouping_config
self._client_ip = client_ip
self._user_agent = user_agent
self._auth = auth
Expand Down Expand Up @@ -435,6 +440,7 @@ def _normalize_impl(self):
client_ip=self._client_ip,
client=self._auth.client if self._auth else None,
key_id=six.text_type(self._key.id) if self._key else None,
grouping_config=self._grouping_config,
protocol_version=six.text_type(self.version) if self.version is not None else None,
stacktrace_frames_hard_limit=settings.SENTRY_STACKTRACE_FRAMES_HARD_LIMIT,
max_stacktrace_frames=settings.SENTRY_MAX_STACKTRACE_FRAMES,
Expand Down Expand Up @@ -626,7 +632,6 @@ def save(self, project_id, raw=False, assume_normalized=False):

transaction_name = data.get('transaction')
logger_name = data.get('logger')
fingerprint = data.get('fingerprint') or ['{{ default }}']
release = data.get('release')
dist = data.get('dist')
environment = data.get('environment')
Expand Down Expand Up @@ -702,9 +707,12 @@ def save(self, project_id, raw=False, assume_normalized=False):
if iface.ephemeral:
data.pop(iface.path, None)

# Put the actual fingerprint back
data['fingerprint'] = fingerprint

# The active grouping config was put into the event in the
# normalize step before. We now also make sure that the
# fingerprint was set to `'{{ default }}' just in case someone
# removed it from the payload. The call to get_hashes will then
# look at `grouping_config` to pick the right paramters.
data['fingerprint'] = data.get('fingerprint') or ['{{ default }}']
hashes = event.get_hashes()
data['hashes'] = hashes

Expand Down
1 change: 1 addition & 0 deletions src/sentry/features/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
default_manager.add('organizations:suggested-commits', OrganizationFeature) # NOQA
default_manager.add('organizations:unreleased-changes', OrganizationFeature) # NOQA
default_manager.add('organizations:grouping-info', OrganizationFeature) # NOQA
default_manager.add('organizations:set-grouping-config', OrganizationFeature) # NOQA

# Project scoped features
default_manager.add('projects:custom-inbound-filters', ProjectFeature) # NOQA
Expand Down
65 changes: 55 additions & 10 deletions src/sentry/grouping/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,61 @@
HASH_RE = re.compile(r'^[0-9a-f]{32}$')


def get_calculated_grouping_variants_for_event(event, config_name=None):
"""Given an event this returns a dictionary of the matching grouping
variants. Checksum and fingerprinting logic are not handled by this
function which is handled by `get_grouping_variants_for_event`.
class ConfigNotFoundException(LookupError):
pass


def get_grouping_config_dict_for_project(project, silent=True):
"""Fetches all the information necessary for grouping from the project
settings. The return value of this is persisted with the event on
ingestion so that the grouping algorithm can be re-run later.
This is called early on in normalization so that everything that is needed
to group the project is pulled into the event.
"""
config_id = project.get_option('sentry:grouping_config')
if config_id is None:
config_id = DEFAULT_CONFIG
else:
try:
CONFIGURATIONS[config_id]
except KeyError:
if not silent:
raise ConfigNotFoundException(config_id)
config_id = DEFAULT_CONFIG

# At a later point we might want to store additional information here
# such as frames that mark the end of a stacktrace and more.
return {
'id': config_id,
}


def get_default_grouping_config_dict():
"""Returns the default grouping config."""
return {
'id': DEFAULT_CONFIG,
}


def load_grouping_config(config_dict=None):
"""Loads the given grouping config."""
if config_dict is None:
config_dict = get_default_grouping_config_dict()
elif 'id' not in config_dict:
raise ValueError('Malformed configuration dictionary')
config_dict = dict(config_dict)
config_id = config_dict.pop('id')
if config_id not in CONFIGURATIONS:
raise ConfigNotFoundException(config_id)
return CONFIGURATIONS[config_id](**config_dict)


def _get_calculated_grouping_variants_for_event(event, config):
winning_strategy = None
precedence_hint = None
per_variant_components = {}

config = CONFIGURATIONS[config_name or DEFAULT_CONFIG]

for strategy in config.iter_strategies():
rv = strategy.get_grouping_component_variants(event, config=config)
for (variant, component) in six.iteritems(rv):
Expand Down Expand Up @@ -56,7 +100,7 @@ def get_calculated_grouping_variants_for_event(event, config_name=None):
return rv


def get_grouping_variants_for_event(event, config_name=None):
def get_grouping_variants_for_event(event, config=None):
"""Returns a dict of all grouping variants for this event."""
# If a checksum is set the only variant that comes back from this
# event is the checksum variant.
Expand Down Expand Up @@ -84,18 +128,19 @@ def get_grouping_variants_for_event(event, config_name=None):

# At this point we need to calculate the default event values. If the
# fingerprint is salted we will wrap it.
components = get_calculated_grouping_variants_for_event(event, config_name)
config = load_grouping_config(config)
components = _get_calculated_grouping_variants_for_event(event, config)
rv = {}

# If the fingerprints are unsalted, we can return them right away.
if defaults_referenced == 1 and len(fingerprint) == 1:
for (key, component) in six.iteritems(components):
rv[key] = ComponentVariant(component)
rv[key] = ComponentVariant(component, config)

# Otherwise we need to salt each of the components.
else:
for (key, component) in six.iteritems(components):
rv[key] = SaltedComponentVariant(fingerprint, component)
rv[key] = SaltedComponentVariant(fingerprint, component, config)

# Ensure we have a fallback hash if nothing else works out
if not any(x.contributes for x in six.itervalues(rv)):
Expand Down
Loading

0 comments on commit 1fcc598

Please sign in to comment.