Skip to content

Commit 3718c39

Browse files
authored
feat(discover2) Make pagination functional again (#14395)
Restore the pagination button behavior and make the oldest/newest links work as intended. We now pass `field` to the API to allow pagination event ids to be constrained by the current event or 'reference' event. We use the group conditions and reference event values to ensure that the next/previous event are in the same 'group' as the modal looks at groupings of events now. Remove the `issue_title` alias. Using/having this alias makes grouping events into errors harder as the title doesn't serve as a grouping value for pagination. By using a simple title we can paginate more easily in the modal view, and we have less complexity overall. Refs SEN-887
1 parent d93ae5c commit 3718c39

File tree

13 files changed

+210
-61
lines changed

13 files changed

+210
-61
lines changed

src/sentry/api/bases/organization_events.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@
88
from sentry.models.project import Project
99

1010

11+
class Direction(object):
12+
NEXT = 0
13+
PREV = 1
14+
15+
1116
class OrganizationEventsEndpointBase(OrganizationEndpoint):
1217
def get_snuba_query_args(self, request, organization, params):
1318
query = request.GET.get("query")
@@ -109,3 +114,36 @@ def prev_event_id(self, request, organization, snuba_args, event):
109114

110115
if prev_event:
111116
return prev_event[1]
117+
118+
def oldest_event_id(self, snuba_args, event):
119+
"""
120+
Returns the oldest event ID if there is a subsequent event matching the
121+
conditions provided
122+
"""
123+
return self._get_terminal_event_id(Direction.PREV, snuba_args, event)
124+
125+
def latest_event_id(self, snuba_args, event):
126+
"""
127+
Returns the latest event ID if there is a newer event matching the
128+
conditions provided
129+
"""
130+
return self._get_terminal_event_id(Direction.NEXT, snuba_args, event)
131+
132+
def _get_terminal_event_id(self, direction, snuba_args, event):
133+
if direction == Direction.NEXT:
134+
time_condition = [["timestamp", ">", event.timestamp]]
135+
orderby = ["-timestamp", "-event_id"]
136+
else:
137+
time_condition = [["timestamp", "<", event.timestamp]]
138+
orderby = ["timestamp", "event_id"]
139+
140+
conditions = snuba_args["conditions"][:]
141+
conditions.extend(time_condition)
142+
143+
result = eventstore.get_events(
144+
conditions=conditions, filter_keys=snuba_args["filter_keys"], orderby=orderby, limit=1
145+
)
146+
if not result:
147+
return None
148+
149+
return result[0].event_id

src/sentry/api/endpoints/organization_event_details.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from enum import Enum
66

77
from sentry.api.bases import OrganizationEventsEndpointBase, OrganizationEventsError, NoProjects
8+
from sentry.api.event_search import get_reference_event_conditions
89
from sentry import eventstore, features
910
from sentry.models import SnubaEvent
1011
from sentry.models.project import Project
@@ -37,11 +38,17 @@ def get(self, request, organization, project_slug, event_id):
3738

3839
# We return the requested event if we find a match regardless of whether
3940
# it occurred within the range specified
40-
event = eventstore.get_event_by_id(project.id, event_id)
41+
event = eventstore.get_event_by_id(project.id, event_id, eventstore.full_columns)
4142

4243
if event is None:
4344
return Response({"detail": "Event not found"}, status=404)
4445

46+
# Scope the pagination related event ids to the current event
47+
# This ensure that if a field list/groupby conditions were provided
48+
# that we constrain related events to the query + current event values
49+
snuba_args["conditions"].extend(
50+
get_reference_event_conditions(snuba_args, event.snuba_data)
51+
)
4552
next_event = eventstore.get_next_event_id(
4653
event, conditions=snuba_args["conditions"], filter_keys=snuba_args["filter_keys"]
4754
)
@@ -52,7 +59,8 @@ def get(self, request, organization, project_slug, event_id):
5259
data = serialize(event)
5360
data["nextEventID"] = next_event[1] if next_event else None
5461
data["previousEventID"] = prev_event[1] if prev_event else None
55-
62+
data["oldestEventID"] = self.oldest_event_id(snuba_args, event)
63+
data["latestEventID"] = self.latest_event_id(snuba_args, event)
5664
data["projectSlug"] = project_slug
5765

5866
return Response(data)

src/sentry/api/event_search.py

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,16 @@
1212
from parsimonious.nodes import Node
1313
from parsimonious.grammar import Grammar, NodeVisitor
1414

15+
from sentry import eventstore
16+
from sentry.models import Project
1517
from sentry.search.utils import (
1618
parse_datetime_range,
1719
parse_datetime_string,
1820
parse_datetime_value,
1921
InvalidQuery,
2022
)
2123
from sentry.utils.dates import to_timestamp
22-
from sentry.utils.snuba import SENTRY_SNUBA_MAP
23-
from sentry.models import Project
24+
from sentry.utils.snuba import SENTRY_SNUBA_MAP, get_snuba_column_name
2425

2526
WILDCARD_CHARS = re.compile(r"[\*]")
2627

@@ -650,7 +651,6 @@ def get_snuba_query_args(query=None, params=None):
650651

651652

652653
FIELD_ALIASES = {
653-
"issue_title": {"aggregations": [["anyHeavy", "title", "issue_title"]]},
654654
"last_seen": {"aggregations": [["max", "timestamp", "last_seen"]]},
655655
"latest_event": {
656656
"aggregations": [
@@ -800,3 +800,44 @@ def resolve_field_list(fields, snuba_args):
800800
"groupby": groupby,
801801
"orderby": orderby,
802802
}
803+
804+
805+
def find_reference_event(snuba_args, reference_event_id):
806+
try:
807+
project_slug, event_id = reference_event_id.split(":")
808+
except ValueError:
809+
raise InvalidSearchQuery("Invalid reference event")
810+
try:
811+
project = Project.objects.get(
812+
slug=project_slug, id__in=snuba_args["filter_keys"]["project_id"]
813+
)
814+
except Project.DoesNotExist:
815+
raise InvalidSearchQuery("Invalid reference event")
816+
817+
reference_event = eventstore.get_event_by_id(project.id, event_id, eventstore.full_columns)
818+
if not reference_event:
819+
raise InvalidSearchQuery("Invalid reference event")
820+
821+
return reference_event
822+
823+
824+
def get_reference_event_conditions(snuba_args, reference_event):
825+
"""
826+
Returns a list of additional conditions/filter_keys to
827+
scope a query by the groupby fields using values from the reference event
828+
829+
This is a key part of pagination in the event details modal and
830+
summary graph navigation.
831+
"""
832+
conditions = []
833+
834+
# If we were given an project/event to use build additional
835+
# conditions using that event and the non-aggregated columns
836+
# we received in the querystring. This lets us find the oldest/newest.
837+
# This only handles simple fields on the snuba_data dict.
838+
for field in snuba_args.get("groupby", []):
839+
prop = get_snuba_column_name(field)
840+
value = reference_event.get(prop, None)
841+
if value:
842+
conditions.append([prop, "=", value])
843+
return conditions

src/sentry/eventstore/base.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ class Columns(Enum):
2323
IP_ADDRESS = "ip_address"
2424
USER_ID = "user_id"
2525
USERNAME = "username"
26+
TRANSACTION = "transaction"
2627

2728

2829
class EventStorage(Service):
@@ -50,6 +51,7 @@ class EventStorage(Service):
5051
Columns.PLATFORM,
5152
Columns.TITLE,
5253
Columns.TYPE,
54+
Columns.TRANSACTION,
5355
# Required to provide snuba-only tags
5456
Columns.TAGS_KEY,
5557
Columns.TAGS_VALUE,

src/sentry/static/sentry/app/views/organizationEventsV2/data.jsx

Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {QueryLink} from './styles';
1717

1818
export const MODAL_QUERY_KEYS = ['eventSlug'];
1919
export const PIN_ICON = `image://${pinIcon}`;
20-
export const AGGREGATE_ALIASES = ['issue_title', 'last_seen', 'latest_event'];
20+
export const AGGREGATE_ALIASES = ['last_seen', 'latest_event'];
2121

2222
export const ALL_VIEWS = deepFreeze([
2323
{
@@ -42,9 +42,9 @@ export const ALL_VIEWS = deepFreeze([
4242
id: 'errors',
4343
name: t('Errors'),
4444
data: {
45-
fields: ['issue_title', 'count(id)', 'count_unique(user)', 'project', 'last_seen'],
45+
fields: ['title', 'count(id)', 'count_unique(user)', 'project', 'last_seen'],
4646
columnNames: ['error', 'events', 'users', 'project', 'last seen'],
47-
sort: ['-last_seen', '-issue_title'],
47+
sort: ['-last_seen', '-title'],
4848
query: 'event.type:error',
4949
},
5050
tags: ['error.type', 'project.name'],
@@ -54,9 +54,9 @@ export const ALL_VIEWS = deepFreeze([
5454
id: 'csp',
5555
name: t('CSP'),
5656
data: {
57-
fields: ['issue_title', 'count(id)', 'count_unique(user)', 'project', 'last_seen'],
57+
fields: ['title', 'count(id)', 'count_unique(user)', 'project', 'last_seen'],
5858
columnNames: ['csp', 'events', 'users', 'project', 'last seen'],
59-
sort: ['-last_seen', '-issue_title'],
59+
sort: ['-last_seen', '-title'],
6060
query: 'event.type:csp',
6161
},
6262
tags: [
@@ -164,11 +164,12 @@ export const SPECIAL_FIELDS = {
164164
transaction: {
165165
sortField: 'transaction',
166166
renderFunc: (data, {location}) => {
167+
const id = data.id || data.latest_event;
167168
const target = {
168169
pathname: location.pathname,
169170
query: {
170171
...location.query,
171-
eventSlug: `${data['project.name']}:${data.latest_event}`,
172+
eventSlug: `${data['project.name']}:${id}`,
172173
},
173174
};
174175
return (
@@ -183,9 +184,10 @@ export const SPECIAL_FIELDS = {
183184
title: {
184185
sortField: 'title',
185186
renderFunc: (data, {location}) => {
187+
const id = data.id || data.latest_event;
186188
const target = {
187189
pathname: location.pathname,
188-
query: {...location.query, eventSlug: `${data['project.name']}:${data.id}`},
190+
query: {...location.query, eventSlug: `${data['project.name']}:${id}`},
189191
};
190192
return (
191193
<Container>
@@ -255,25 +257,6 @@ export const SPECIAL_FIELDS = {
255257
return <QueryLink to={target}>{badge}</QueryLink>;
256258
},
257259
},
258-
issue_title: {
259-
sortField: 'issue_title',
260-
renderFunc: (data, {location}) => {
261-
const target = {
262-
pathname: location.pathname,
263-
query: {
264-
...location.query,
265-
eventSlug: `${data['project.name']}:${data.latest_event}`,
266-
},
267-
};
268-
return (
269-
<Container>
270-
<Link css={overflowEllipsis} to={target} aria-label={data.issue_title}>
271-
{data.issue_title}
272-
</Link>
273-
</Container>
274-
);
275-
},
276-
},
277260
last_seen: {
278261
sortField: 'last_seen',
279262
renderFunc: data => {

src/sentry/static/sentry/app/views/organizationEventsV2/eventModalContent.jsx

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ import ModalPagination from './modalPagination';
1717
import ModalLineGraph from './modalLineGraph';
1818
import RelatedEvents from './relatedEvents';
1919
import TagsTable from './tagsTable';
20-
import {AGGREGATE_ALIASES} from './data';
2120
import TransanctionView from './transactionView';
21+
import {hasAggregateField} from './utils';
2222

2323
/**
2424
* Render the columns and navigation elements inside the event modal view.
@@ -27,10 +27,8 @@ import TransanctionView from './transactionView';
2727
const EventModalContent = props => {
2828
const {event, projectId, organization, location, view} = props;
2929

30-
// Known aggregate aliases and functions indicated grouped views.
31-
const isGroupedView = !!view.data.fields.find(
32-
field => AGGREGATE_ALIASES.includes(field) || field.match(/[a-z_]+\([a-z_\.]+\)/)
33-
);
30+
// Having an aggregate field means we want to show pagination/graphs
31+
const isGroupedView = hasAggregateField(view);
3432
const eventJsonUrl = `/api/0/projects/${organization.slug}/${projectId}/events/${
3533
event.eventID
3634
}/json/`;
@@ -39,9 +37,7 @@ const EventModalContent = props => {
3937
<ColumnGrid>
4038
<HeaderBox>
4139
<EventHeader event={event} />
42-
{isGroupedView && (
43-
<ModalPagination view={view} event={event} location={location} />
44-
)}
40+
{isGroupedView && <ModalPagination event={event} location={location} />}
4541
{isGroupedView &&
4642
getDynamicText({
4743
value: (

src/sentry/static/sentry/app/views/organizationEventsV2/modalPagination.jsx

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,8 @@ function buildTargets(event, location) {
2222
const urlMap = {
2323
previous: event.previousEventID,
2424
next: event.nextEventID,
25-
// TODO(mark) Make latest, oldest work once we have new endpoints.
26-
// `${event.eventID}:latest`,
27-
latest: null,
28-
oldest: null,
25+
oldest: event.oldestEventID,
26+
latest: event.latestEventID,
2927
};
3028

3129
const links = {};

src/sentry/static/sentry/app/views/organizationEventsV2/utils.jsx

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import {partial, pick, get} from 'lodash';
22

33
import {DEFAULT_PER_PAGE} from 'app/constants';
44
import {URL_PARAM} from 'app/constants/globalSelectionHeader';
5-
import {ALL_VIEWS, SPECIAL_FIELDS, FIELD_FORMATTERS} from './data';
5+
import {ALL_VIEWS, AGGREGATE_ALIASES, SPECIAL_FIELDS, FIELD_FORMATTERS} from './data';
66

77
/**
88
* Given a view id, return the corresponding view object
@@ -15,9 +15,25 @@ export function getCurrentView(requestedView) {
1515
return ALL_VIEWS.find(view => view.id === requestedView) || ALL_VIEWS[0];
1616
}
1717

18+
/**
19+
* Takes a view and determines if there are any aggregate fields in it.
20+
*
21+
* TODO(mark) This function should be part of an EventView abstraction
22+
*
23+
* @param {Object} view
24+
* @returns {Boolean}
25+
*/
26+
export function hasAggregateField(view) {
27+
return view.data.fields.some(
28+
field => AGGREGATE_ALIASES.includes(field) || field.match(/[a-z_]+\([a-z_\.]+\)/)
29+
);
30+
}
31+
1832
/**
1933
* Takes a view and converts it into the format required for the events API
2034
*
35+
* TODO(mark) This function should be part of an EventView abstraction
36+
*
2137
* @param {Object} view
2238
* @returns {Object}
2339
*/
@@ -51,6 +67,8 @@ export function getQuery(view, location) {
5167
* Generate a querystring based on the view defaults, current
5268
* location and any additional parameters
5369
*
70+
* TODO(mark) This function should be part of an EventView abstraction
71+
*
5472
* @param {Object} view defaults containing `.data.query`
5573
* @param {Location} browser location
5674
* @param {Object} additional parameters to merge into the query string.

src/sentry/utils/snuba.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
"issue.id": "issue",
6161
"timestamp": "timestamp",
6262
"time": "time",
63+
"transaction": "transaction",
6364
# We support type as both tag and a real column
6465
"event.type": "type",
6566
# user

0 commit comments

Comments
 (0)