Compare commits

..

1 Commits

Author SHA1 Message Date
Aditya Singh
c14d256e6f feat(traces-explorer): track how users open the trace details page (#13420)
Some checks are pending
build-staging / prepare (push) Waiting to run
build-staging / js-build (push) Blocked by required conditions
build-staging / go-build (push) Blocked by required conditions
build-staging / staging (push) Blocked by required conditions
cacheci / tests (push) Waiting to run
Release Drafter / update_release_draft (push) Waiting to run
#### Description

- A row click in the traces List view now opens the span drawer. We need
to know how users still reach the trace details page.
- The explorer now logs `Traces Explorer: Open trace clicked` on each
click that opens the trace details page.
- The event has two attributes:
- `panelType`: `list` or `trace`. These are the same values as `Traces
Explorer: Data present`.
  - `source`: `row`, `row_icon`, `trace_id` or `span_drawer`.

#### Issues closed by this PR

Part of https://github.com/SigNoz/events-pod/issues/195

#### Additional Information

- Stacked on #13411.
- The event does not fire when a user opens the trace_id link from the
right-click menu. The browser does not tell the page which menu item the
user picks.
- Entry points outside the traces explorer do not log this event.
2026-11-06 05:15:33 +00:00
20 changed files with 243 additions and 470 deletions

View File

@@ -10088,10 +10088,8 @@ components:
having:
allOf:
- $ref: '#/components/schemas/Querybuildertypesv5Having'
description: A filter on the formula values, referencing the formula by
its name or as __result, for example F1 > 0. A time series keeps only
the points that match and drops a series with none left; a scalar result
keeps only the rows that match.
description: Accepted but currently ignored; the computed values are returned
unfiltered.
legend:
description: The display name template for each series; {{field}} placeholders
are replaced by label values.

View File

@@ -7046,7 +7046,7 @@ export interface Querybuildertypesv5QueryBuilderFormulaDTO {
*/
functions?: Querybuildertypesv5FunctionDTO[] | null;
/**
* @description A filter on the formula values, referencing the formula by its name or as __result, for example F1 > 0. A time series keeps only the points that match and drops a series with none left; a scalar result keeps only the rows that match.
* @description Accepted but currently ignored; the computed values are returned unfiltered.
*/
having?: Querybuildertypesv5HavingDTO;
/**

View File

@@ -1,3 +1,9 @@
import { logEventMock } from '__tests__/logEventMock';
import { PANEL_TYPES } from 'constants/queryBuilder';
import {
OpenTraceSource,
TracesExplorerEvents,
} from 'container/TracesExplorer/events';
import { getCurrentLocation } from 'lib/router/navigation';
import { server } from 'mocks-server/server';
import { screen, userEvent, waitFor, within } from 'tests/test-utils';
@@ -44,6 +50,12 @@ jest.mock('container/OptionsMenu/useOptionsMenu', () => ({
}),
}));
const ROW_TEST_ID = `traces-table-row-${listRows[0].data.span_id}`;
// The row click handler sits on each cell, so a click on the <tr> does nothing.
const getRowCell = (row: HTMLElement): HTMLElement =>
within(row).getAllByRole('cell')[0];
describe('Traces ListView - span drawer navigation', () => {
beforeEach(() => {
mockSuccess();
@@ -134,6 +146,63 @@ describe('Traces ListView - span drawer navigation', () => {
);
expect(getCurrentLocation().pathname).toBe('/traces-explorer');
expect(screen.queryByTestId('span-details-drawer')).not.toBeInTheDocument();
expect(logEventMock).toHaveBeenCalledWith(
TracesExplorerEvents.OpenTraceClicked,
{
panelType: PANEL_TYPES.LIST,
source: OpenTraceSource.RowIcon,
},
);
});
it('opens the trace page in a new tab from the drawer', async () => {
const open = jest.spyOn(window, 'open').mockImplementation(() => null);
const user = userEvent.setup();
renderListView();
await user.click(getRowCell(await screen.findByTestId(ROW_TEST_ID)));
expect(logEventMock).not.toHaveBeenCalledWith(
TracesExplorerEvents.OpenTraceClicked,
{ panelType: PANEL_TYPES.LIST, source: OpenTraceSource.Row },
);
await user.click(await screen.findByTestId('span-drawer-view-full-trace'));
expect(open).toHaveBeenCalledWith(
expect.stringMatching(/\/trace\/0000000000000000344ded1387b08a7e\?/),
'_blank',
);
expect(logEventMock).toHaveBeenCalledWith(
TracesExplorerEvents.OpenTraceClicked,
{
panelType: PANEL_TYPES.LIST,
source: OpenTraceSource.SpanDrawer,
},
);
});
it('opens the trace page in a new tab on a Cmd-click without opening the drawer', async () => {
const open = jest.spyOn(window, 'open').mockImplementation(() => null);
const user = userEvent.setup();
renderListView();
const cell = getRowCell(await screen.findByTestId(ROW_TEST_ID));
await user.keyboard('{Meta>}');
await user.click(cell);
await user.keyboard('{/Meta}');
expect(open).toHaveBeenCalledWith(
expect.stringMatching(/\/trace\/0000000000000000344ded1387b08a7e\?/),
'_blank',
'noopener',
);
expect(screen.queryByTestId('span-details-drawer')).not.toBeInTheDocument();
expect(logEventMock).toHaveBeenCalledWith(
TracesExplorerEvents.OpenTraceClicked,
{
panelType: PANEL_TYPES.LIST,
source: OpenTraceSource.Row,
},
);
});
it('shows the row icon only on the hovered copy of a duplicate span', async () => {

View File

@@ -146,7 +146,9 @@ function ListView({
(field) => field.name !== TIMESTAMP_FIELD.name,
),
];
const fieldColumns = fields.map((field) => getFieldColumn(field));
const fieldColumns = fields.map((field) =>
getFieldColumn(field, PANEL_TYPES.LIST),
);
return [openTraceColumn, ...fieldColumns];
}, [options?.selectColumns, openTraceColumn]);

View File

@@ -2,7 +2,12 @@ import { MouseEvent } from 'react';
import { CornerUpRight } from '@signozhq/icons';
import { Button } from '@signozhq/ui/button';
import { Tooltip } from '@signozhq/ui/tooltip';
import { PANEL_TYPES } from 'constants/queryBuilder';
import { useIsRowHovered } from 'components/TanStackTableView/TanStackTableStateContext';
import {
logOpenTraceClicked,
OpenTraceSource,
} from 'container/TracesExplorer/events';
import { getTraceLink } from 'container/TracesExplorer/ListView/utils';
import { TracesTableRow } from 'container/TracesExplorer/TracesTable/getFieldColumn';
import { openInNewTab } from 'utils/navigation';
@@ -23,6 +28,7 @@ function OpenTraceButton({
const handleClick = (event: MouseEvent): void => {
// The row click would open the drawer instead.
event.stopPropagation();
logOpenTraceClicked(PANEL_TYPES.LIST, OpenTraceSource.RowIcon);
openInNewTab(getTraceLink(row));
};

View File

@@ -4,7 +4,12 @@ import { ButtonGroup } from '@signozhq/ui/button-group';
import { ChevronDown, ChevronUp, CornerUpRight } from '@signozhq/icons';
import { useGetTraceSummary } from 'api/generated/services/tracedetail';
import { HeaderAction } from 'components/DetailsPanel';
import { PANEL_TYPES } from 'constants/queryBuilder';
import type { UseRowNavigationResult } from 'components/TanStackTableView/useRowNavigation';
import {
logOpenTraceClicked,
OpenTraceSource,
} from 'container/TracesExplorer/events';
import { getTraceLink } from 'container/TracesExplorer/ListView/utils';
import { TracesTableRow } from 'container/TracesExplorer/TracesTable/getFieldColumn';
import { SpanDetailVariant } from 'pages/TraceDetailsV3/SpanDetailsPanel/constants';
@@ -48,7 +53,10 @@ function SpanDrawer({
color="secondary"
size="md"
prefix={<CornerUpRight />}
onClick={(): void => openInNewTab(getTraceLink(row))}
onClick={(): void => {
logOpenTraceClicked(PANEL_TYPES.LIST, OpenTraceSource.SpanDrawer);
openInNewTab(getTraceLink(row));
}}
testId="span-drawer-view-full-trace"
>
View full trace

View File

@@ -4,7 +4,12 @@ import { Minus } from '@signozhq/icons';
import { Badge } from '@signozhq/ui/badge';
import TanStackTable from 'components/TanStackTableView';
import { DATE_TIME_FORMATS } from 'constants/dateTimeFormats';
import { PANEL_TYPES } from 'constants/queryBuilder';
import ROUTES from 'constants/routes';
import {
logOpenTraceClicked,
OpenTraceSource,
} from 'container/TracesExplorer/events';
import { getMs } from 'utils/timeUtils';
import { useTimezone } from 'providers/Timezone';
@@ -21,9 +26,10 @@ import styles from './FieldCell.module.scss';
type FieldCellProps = {
name: string;
value: unknown;
panelType: PANEL_TYPES;
};
function FieldCell({ name, value }: FieldCellProps): JSX.Element {
function FieldCell({ name, value, panelType }: FieldCellProps): JSX.Element {
const { formatTimezoneAdjustedTimestamp } = useTimezone();
if (TIMESTAMP_FIELD_NAMES.has(name)) {
@@ -54,7 +60,15 @@ function FieldCell({ name, value }: FieldCellProps): JSX.Element {
<AppLink
to={buildRoutePath(ROUTES.TRACE_DETAIL, { id: text })}
data-testid="trace-id"
onClick={(e): void => e.stopPropagation()}
onClick={(e): void => {
e.stopPropagation();
logOpenTraceClicked(panelType, OpenTraceSource.TraceId);
}}
onAuxClick={(e): void => {
if (e.button === 1) {
logOpenTraceClicked(panelType, OpenTraceSource.TraceId);
}
}}
>
{text}
</AppLink>

View File

@@ -2,6 +2,7 @@ import { useCallback, useRef } from 'react';
import { useAppNavigate } from 'lib/router/useAppNavigate';
import ErrorInPlace from 'components/ErrorInPlace/ErrorInPlace';
import TanStackTable from 'components/TanStackTableView';
import { PANEL_TYPES } from 'constants/queryBuilder';
import type {
CellTypographySize,
TableColumnDef,
@@ -10,6 +11,10 @@ import { useRecoverFromEmptyPage } from 'components/TanStackTableView/useRecover
import EmptyLogsSearch from 'container/EmptyLogsSearch/EmptyLogsSearch';
import NoLogs from 'container/NoLogs/NoLogs';
import { TracesLoading } from 'container/TracesExplorer/TraceLoading/TraceLoading';
import {
logOpenTraceClicked,
OpenTraceSource,
} from 'container/TracesExplorer/events';
import APIError from 'types/api/error';
import { DataSource, PanelTypeKeys } from 'types/common/queryBuilder';
import { noop } from 'lodash-es';
@@ -101,16 +106,18 @@ function TracesTable({
onRowClick(row);
return;
}
logOpenTraceClicked(PANEL_TYPES[panelType], OpenTraceSource.Row);
navigate(getRowHref(row));
},
[onRowClick, navigate, getRowHref],
[onRowClick, navigate, getRowHref, panelType],
);
const handleRowClickNewTab = useCallback(
(row: TracesTableRow): void => {
logOpenTraceClicked(PANEL_TYPES[panelType], OpenTraceSource.Row);
window.open(getAbsoluteUrl(getRowHref(row)), '_blank', 'noopener');
},
[getRowHref],
[getRowHref, panelType],
);
return (

View File

@@ -2,6 +2,7 @@ import { TelemetryFieldKey } from 'api/v5/v5';
import type { TableColumnDef } from 'components/TanStackTableView/types';
import { buildCompositeKey } from 'container/OptionsMenu/utils';
import { getSpanAttribute } from 'pages/TraceDetailsV3/utils';
import { PANEL_TYPES } from 'constants/queryBuilder';
import { TIMESTAMP_FIELD_NAMES } from './constants';
import FieldCell from './FieldCell';
@@ -10,6 +11,7 @@ export type TracesTableRow = { id: string } & Record<string, unknown>;
export function getFieldColumn(
field: TelemetryFieldKey,
panelType: PANEL_TYPES,
): TableColumnDef<TracesTableRow> {
const { name, fieldContext, fieldDataType } = field;
const isTimestamp = TIMESTAMP_FIELD_NAMES.has(name);
@@ -25,6 +27,8 @@ export function getFieldColumn(
enableRemove: !isTimestamp,
canBeHidden: !isTimestamp,
width: { min: 192 },
cell: ({ value }): JSX.Element => <FieldCell name={name} value={value} />,
cell: ({ value }): JSX.Element => (
<FieldCell name={name} value={value} panelType={panelType} />
),
};
}

View File

@@ -0,0 +1,93 @@
import { logEventMock } from '__tests__/logEventMock';
import { PANEL_TYPES } from 'constants/queryBuilder';
import {
OpenTraceSource,
TracesExplorerEvents,
} from 'container/TracesExplorer/events';
import { getCurrentLocation } from 'lib/router/navigation';
import { server } from 'mocks-server/server';
import { rest } from 'msw';
import { screen, userEvent, waitFor, within } from 'tests/test-utils';
import {
groupedResponse,
groupedRows,
QUERY_RANGE_URL,
renderTracesView,
} from './testUtils';
const TRACE_ID = '0000000000000000344ded1387b08a7e';
const ROW_TEST_ID = `traces-table-row-${TRACE_ID}`;
describe('TracesView - open trace clicked', () => {
beforeEach(() => {
server.use(
rest.post(QUERY_RANGE_URL, (_req, res, ctx) =>
res(ctx.status(200), ctx.json(groupedResponse(groupedRows))),
),
);
});
afterEach(() => {
server.resetHandlers();
});
it('logs a row click that opens the trace page', async () => {
const user = userEvent.setup();
renderTracesView();
const row = await screen.findByTestId(ROW_TEST_ID);
// The row click handler sits on each cell, so a click on the <tr> does nothing.
await user.click(within(row).getAllByRole('cell')[0]);
await waitFor(() =>
expect(getCurrentLocation().pathname).toBe(`/trace/${TRACE_ID}`),
);
expect(logEventMock).toHaveBeenCalledWith(
TracesExplorerEvents.OpenTraceClicked,
{
panelType: PANEL_TYPES.TRACE,
source: OpenTraceSource.Row,
},
);
});
it('logs a trace_id link click', async () => {
const user = userEvent.setup();
renderTracesView();
const row = await screen.findByTestId(ROW_TEST_ID);
await user.click(within(row).getByTestId('trace-id'));
expect(logEventMock).toHaveBeenCalledWith(
TracesExplorerEvents.OpenTraceClicked,
{
panelType: PANEL_TYPES.TRACE,
source: OpenTraceSource.TraceId,
},
);
expect(logEventMock).not.toHaveBeenCalledWith(
TracesExplorerEvents.OpenTraceClicked,
{ panelType: PANEL_TYPES.TRACE, source: OpenTraceSource.Row },
);
});
it('logs a middle click on the trace_id link', async () => {
const user = userEvent.setup();
renderTracesView();
const row = await screen.findByTestId(ROW_TEST_ID);
await user.pointer({
keys: '[MouseMiddle]',
target: within(row).getByTestId('trace-id'),
});
expect(logEventMock).toHaveBeenCalledWith(
TracesExplorerEvents.OpenTraceClicked,
{
panelType: PANEL_TYPES.TRACE,
source: OpenTraceSource.TraceId,
},
);
});
});

View File

@@ -1,4 +1,5 @@
import { TelemetryFieldKey } from 'api/v5/v5';
import { PANEL_TYPES } from 'constants/queryBuilder';
import type { TableColumnDef } from 'components/TanStackTableView/types';
import {
getFieldColumn,
@@ -15,7 +16,7 @@ const TRACE_FIELDS = [
export const columns: TableColumnDef<TracesTableRow>[] = TRACE_FIELDS.map(
(field) => ({
...getFieldColumn(field),
...getFieldColumn(field, PANEL_TYPES.TRACE),
enableRemove: false,
canBeHidden: false,
}),

View File

@@ -0,0 +1,20 @@
import logEvent from 'api/common/logEvent';
import { PANEL_TYPES } from 'constants/queryBuilder';
export enum TracesExplorerEvents {
OpenTraceClicked = 'Traces Explorer: Open trace clicked',
}
export enum OpenTraceSource {
Row = 'row',
RowIcon = 'row_icon',
TraceId = 'trace_id',
SpanDrawer = 'span_drawer',
}
export function logOpenTraceClicked(
panelType: PANEL_TYPES,
source: OpenTraceSource,
): void {
void logEvent(TracesExplorerEvents.OpenTraceClicked, { panelType, source });
}

View File

@@ -415,12 +415,6 @@ func (q *querier) processTimeSeriesFormula(
return nil
}
formulaSeries, err = applyFormulaHaving(formula, formulaSeries)
if err != nil {
q.logger.ErrorContext(ctx, "failed to apply formula having", errors.Attr(err), slog.String("formula", formula.Name))
return nil
}
// Create result for formula
formulaResult := &qbtypes.TimeSeriesData{
QueryName: formula.Name,
@@ -656,12 +650,6 @@ func (q *querier) processScalarFormula(
return nil
}
formulaSeries, err = applyFormulaHaving(formula, formulaSeries)
if err != nil {
q.logger.ErrorContext(ctx, "failed to apply formula having", errors.Attr(err), slog.String("formula", formula.Name))
return nil
}
// Apply ordering (and limit) before converting to scalar format.
formulaSeries = qbtypes.ApplySeriesLimit(formulaSeries, formula.Order, formula.Limit)
@@ -712,35 +700,6 @@ func (q *querier) processScalarFormula(
}
}
// applyFormulaHaving keeps the values the formula having accepts, as SQL HAVING
// keeps rows for a builder query, so a series with no values left is removed. It
// runs before functions, order, and limit, as HAVING runs before them in SQL.
func applyFormulaHaving(formula qbtypes.QueryBuilderFormula, series []*qbtypes.TimeSeries) ([]*qbtypes.TimeSeries, error) {
if formula.Having == nil || strings.TrimSpace(formula.Having.Expression) == "" {
return series, nil
}
keep, err := querybuilder.CompileFormulaHaving(formula.Name, formula.Having.Expression)
if err != nil {
return nil, err
}
kept := series[:0]
for _, s := range series {
values := s.Values[:0]
for _, value := range s.Values {
if keep(value.Value) {
values = append(values, value)
}
}
if len(values) > 0 {
s.Values = values
kept = append(kept, s)
}
}
return kept, nil
}
// filterDisabledQueries removes results for disabled queries.
func (q *querier) filterDisabledQueries(results map[string]*qbtypes.Result, req *qbtypes.QueryRangeRequest) map[string]*qbtypes.Result {
filtered := make(map[string]*qbtypes.Result)

View File

@@ -212,14 +212,6 @@ func (q *querier) buildQueries(
}
switch query.Type {
case qbtypes.QueryTypeFormula:
// the querier evaluates a formula after its queries run, so a bad
// having must fail here, before they run
if formula, ok := query.Spec.(qbtypes.QueryBuilderFormula); ok && formula.Having != nil && strings.TrimSpace(formula.Having.Expression) != "" {
if _, err := querybuilder.CompileFormulaHaving(formula.Name, formula.Having.Expression); err != nil {
return nil, nil, err
}
}
case qbtypes.QueryTypePromQL:
promQuery, ok := query.Spec.(qbtypes.PromQuery)
if !ok {

View File

@@ -1,166 +0,0 @@
package querybuilder
import (
"math"
"slices"
"strconv"
grammar "github.com/SigNoz/signoz/pkg/parser/havingexpression/grammar"
qbtypes "github.com/SigNoz/signoz/pkg/types/querybuildertypes/querybuildertypesv5"
)
// CompileFormulaHaving turns the having expression of a formula into a filter on
// its values. The querier computes a formula in Go after ClickHouse returns, so the
// having cannot run as SQL; it gets the same validation and messages as a
// builder-query having, with the formula name and __result as the only references.
func CompileFormulaHaving(formulaName, expression string) (func(value float64) bool, error) {
rewriter := &HavingExpressionRewriter{columnMap: map[string]string{
formulaName: qbtypes.DefaultOrderByKey,
qbtypes.DefaultOrderByKey: qbtypes.DefaultOrderByKey,
}}
_, tree, err := rewriter.parseAndValidate(expression)
if err != nil {
return nil, err
}
return compileHavingOr(tree.Expression().OrExpression()), nil
}
func compileHavingOr(ctx grammar.IOrExpressionContext) func(float64) bool {
andExpressions := ctx.AllAndExpression()
conditions := make([]func(float64) bool, len(andExpressions))
for i, andExpression := range andExpressions {
conditions[i] = compileHavingAnd(andExpression)
}
return func(value float64) bool {
for _, condition := range conditions {
if condition(value) {
return true
}
}
return false
}
}
func compileHavingAnd(ctx grammar.IAndExpressionContext) func(float64) bool {
primaries := ctx.AllPrimary()
conditions := make([]func(float64) bool, len(primaries))
for i, primary := range primaries {
conditions[i] = compileHavingPrimary(primary)
}
return func(value float64) bool {
for _, condition := range conditions {
if !condition(value) {
return false
}
}
return true
}
}
func compileHavingPrimary(ctx grammar.IPrimaryContext) func(float64) bool {
var condition func(float64) bool
if ctx.OrExpression() != nil {
condition = compileHavingOr(ctx.OrExpression())
} else {
condition = compileHavingComparison(ctx.Comparison())
}
if ctx.NOT() == nil {
return condition
}
return func(value float64) bool { return !condition(value) }
}
func compileHavingComparison(ctx grammar.IComparisonContext) func(float64) bool {
left := compileHavingOperand(ctx.Operand(0))
if ctx.IN() != nil {
signedNumbers := ctx.InList().AllSignedNumber()
list := make([]float64, len(signedNumbers))
for i, signedNumber := range signedNumbers {
list[i] = parseHavingNumber(signedNumber.GetText())
}
notIn := ctx.NOT() != nil
return func(value float64) bool { return slices.Contains(list, left(value)) != notIn }
}
right := compileHavingOperand(ctx.Operand(1))
compOp := ctx.CompOp()
switch {
case compOp.EQUALS() != nil:
return func(value float64) bool { return left(value) == right(value) }
case compOp.NOT_EQUALS() != nil, compOp.NEQ() != nil:
return func(value float64) bool { return left(value) != right(value) }
case compOp.LT() != nil:
return func(value float64) bool { return left(value) < right(value) }
case compOp.LE() != nil:
return func(value float64) bool { return left(value) <= right(value) }
case compOp.GT() != nil:
return func(value float64) bool { return left(value) > right(value) }
default:
return func(value float64) bool { return left(value) >= right(value) }
}
}
func compileHavingOperand(ctx grammar.IOperandContext) func(float64) float64 {
if ctx.Operand() == nil {
return compileHavingTerm(ctx.Term())
}
left := compileHavingOperand(ctx.Operand())
right := compileHavingTerm(ctx.Term())
if ctx.MINUS() != nil {
return func(value float64) float64 { return left(value) - right(value) }
}
return func(value float64) float64 { return left(value) + right(value) }
}
func compileHavingTerm(ctx grammar.ITermContext) func(float64) float64 {
if ctx.Term() == nil {
return compileHavingFactor(ctx.Factor())
}
left := compileHavingTerm(ctx.Term())
right := compileHavingFactor(ctx.Factor())
switch {
case ctx.SLASH() != nil:
return func(value float64) float64 { return left(value) / right(value) }
case ctx.PERCENT() != nil:
return func(value float64) float64 { return math.Mod(left(value), right(value)) }
default:
return func(value float64) float64 { return left(value) * right(value) }
}
}
func compileHavingFactor(ctx grammar.IFactorContext) func(float64) float64 {
if ctx.Factor() != nil {
inner := compileHavingFactor(ctx.Factor())
if ctx.MINUS() != nil {
return func(value float64) float64 { return -inner(value) }
}
return inner
}
if ctx.Operand() != nil {
return compileHavingOperand(ctx.Operand())
}
if number := ctx.Atom().NUMBER(); number != nil {
constant := parseHavingNumber(number.GetText())
return func(float64) float64 { return constant }
}
// validation rejects function calls, strings, and unknown names, so the atom
// names the formula value
return func(value float64) float64 { return value }
}
// ParseFloat reads every form the lexer admits as a number; a literal outside the
// float64 range becomes the nearest value ParseFloat returns.
func parseHavingNumber(text string) float64 {
number, _ := strconv.ParseFloat(text, 64)
return number
}

View File

@@ -1,85 +0,0 @@
package querybuilder
import (
"testing"
"github.com/SigNoz/signoz/pkg/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestCompileFormulaHaving_Matches(t *testing.T) {
values := []float64{-2, 0, 1, 2.5, 3, 10}
testCases := []struct {
name string
expression string
want []float64
}{
{name: "Name_GreaterThan", expression: "F1 > 2", want: []float64{2.5, 3, 10}},
{name: "Result_LessOrEqual", expression: "__result <= 1", want: []float64{-2, 0, 1}},
{name: "DoubleEquals", expression: "F1 == 3", want: []float64{3}},
{name: "AngleNotEquals", expression: "F1 <> 0", want: []float64{-2, 1, 2.5, 3, 10}},
{name: "In_Parentheses", expression: "F1 IN (0, 3, -2)", want: []float64{-2, 0, 3}},
{name: "NotIn_Brackets", expression: "F1 NOT IN [0, 3, -2]", want: []float64{1, 2.5, 10}},
{name: "AndBindsTighterThanOr", expression: "F1 > 0 AND F1 < 3 OR F1 = -2", want: []float64{-2, 1, 2.5}},
{name: "ImplicitAnd", expression: "F1 > 0 F1 < 3", want: []float64{1, 2.5}},
{name: "LowercaseKeywords", expression: "F1 > 2 and not F1 = 10", want: []float64{2.5, 3}},
{name: "NotParenthesized", expression: "NOT (F1 >= 1 AND F1 <= 3)", want: []float64{-2, 0, 10}},
{name: "Arithmetic", expression: "F1 * 2 - 1 > 4", want: []float64{3, 10}},
{name: "Arithmetic_Parentheses", expression: "(F1 + 2) / 2 >= 2", want: []float64{2.5, 3, 10}},
{name: "Modulo", expression: "F1 % 2 = 1", want: []float64{1, 3}},
{name: "UnaryMinus", expression: "-F1 > 1", want: []float64{-2}},
{name: "Number_Exponent", expression: "F1 >= 1e1", want: []float64{10}},
{name: "Number_LeadingDot", expression: "F1 < .5", want: []float64{-2, 0}},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
keep, err := CompileFormulaHaving("F1", testCase.expression)
require.NoError(t, err)
require.NotNil(t, keep)
got := []float64{}
for _, value := range values {
if keep(value) {
got = append(got, value)
}
}
assert.Equal(t, testCase.want, got)
})
}
}
func TestCompileFormulaHaving_Invalid(t *testing.T) {
testCases := []struct {
name string
expression string
wantErrMsg string
wantAdditional []string
}{
{
name: "QueryName_NotAReference",
expression: "A > 0",
wantErrMsg: "Invalid references in `Having` expression: [A]",
wantAdditional: []string{"Valid references are: [F1, __result]"},
},
{
name: "Aggregation_NotAReference",
expression: "count() > 0",
wantErrMsg: "Invalid references in `Having` expression: [count]",
wantAdditional: []string{"Valid references are: [F1, __result]"},
},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
keep, err := CompileFormulaHaving("F1", testCase.expression)
require.Error(t, err)
assert.Nil(t, keep)
assert.True(t, errors.Ast(err, errors.TypeInvalidInput))
assert.ErrorContains(t, err, testCase.wantErrMsg)
assert.Equal(t, testCase.wantAdditional, additionalMessages(err))
})
}
}

View File

@@ -237,13 +237,7 @@ func (v *havingExpressionRewriteVisitor) visitIdentifier(ctx grammar.IIdentifier
return name
}
func (r *HavingExpressionRewriter) rewriteAndValidate(expression string) (string, error) {
rewritten, _, err := r.parseAndValidate(expression)
return rewritten, err
}
// parseAndValidate is the single-pass implementation used by all RewriteFor* methods.
// It also returns the parse tree, which CompileFormulaHaving evaluates in Go.
// rewriteAndValidate is the single-pass implementation used by all RewriteFor* methods.
//
// Validation layers:
// 1. The visitor runs on the parse tree, rewriting and collecting invalid references.
@@ -254,7 +248,7 @@ func (r *HavingExpressionRewriter) rewriteAndValidate(expression string) (string
// 2. String literals in comparison-operand position → descriptive error
// ("aggregator results are numeric").
// 3. ANTLR syntax errors → error with messages referencing the original token names.
func (r *HavingExpressionRewriter) parseAndValidate(expression string) (string, grammar.IQueryContext, error) {
func (r *HavingExpressionRewriter) rewriteAndValidate(expression string) (string, error) {
original := strings.TrimSpace(expression)
// Parse the expression once.
@@ -287,7 +281,7 @@ func (r *HavingExpressionRewriter) parseAndValidate(expression string) (string,
// This is checked before invalid references so that "contains string literals" takes
// priority when a bare string literal is also an unresolvable operand.
if v.hasStringLiteral {
return "", nil, errors.NewInvalidInputf(
return "", errors.NewInvalidInputf(
errors.CodeInvalidInput,
"`Having` expression contains string literals",
).WithAdditional("Aggregator results are numeric")
@@ -323,7 +317,7 @@ func (r *HavingExpressionRewriter) parseAndValidate(expression string) (string,
).WithAdditional(
"Valid references are: [" + strings.Join(validKeys, ", ") + "]",
).WithSuggestions(suggestions...)
return "", nil, havingErr
return "", havingErr
}
// Layer 3 – ANTLR syntax errors. We parse the original expression, so error messages
@@ -349,12 +343,12 @@ func (r *HavingExpressionRewriter) parseAndValidate(expression string) (string,
return havingSuggestion(allSyntaxErrors[0], original)
})
return "", nil, havingErr.WithSuggestiveAdditional(msgs[0], suggestions...)
return "", havingErr.WithSuggestiveAdditional(msgs[0], suggestions...)
}
return "", nil, havingErr.WithAdditional(msgs...)
return "", havingErr.WithAdditional(msgs...)
}
return result, tree, nil
return result, nil
}
// havingSuggestion returns a corrected expression string to show as a suggestion when

View File

@@ -26,7 +26,7 @@ type QueryBuilderFormula struct {
Limit int `json:"limit,omitempty" description:"The maximum number of series or rows to return. Unlimited when 0."`
Having *Having `json:"having,omitempty" description:"A filter on the formula values, referencing the formula by its name or as __result, for example F1 > 0. A time series keeps only the points that match and drops a series with none left; a scalar result keeps only the rows that match."`
Having *Having `json:"having,omitempty" description:"Accepted but currently ignored; the computed values are returned unfiltered."`
Functions []Function `json:"functions,omitzero" description:"Post-processing functions applied in order to each result series."`

View File

@@ -348,7 +348,6 @@ def build_formula_query(
disabled: bool = False,
order: list[dict] | None = None,
limit: int | None = None,
having_expression: str | None = None,
) -> dict:
spec: dict[str, Any] = {
"name": name,
@@ -363,8 +362,6 @@ def build_formula_query(
spec["order"] = order
if limit is not None:
spec["limit"] = limit
if having_expression:
spec["having"] = {"expression": having_expression}
return {"type": "builder_formula", "spec": spec}

View File

@@ -2,8 +2,6 @@ from collections.abc import Callable
from datetime import UTC, datetime, timedelta
from http import HTTPStatus
import pytest
from fixtures import types
from fixtures.auth import USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD
from fixtures.logs import Logs
@@ -14,9 +12,6 @@ from fixtures.querier import (
build_order_by,
build_scalar_query,
find_named_result,
get_all_series,
get_error_message,
index_series_by_label,
make_query_request,
)
@@ -196,138 +191,3 @@ def test_logs_formula_orderby_and_limit(
assert len(f3_services) == 3, f"F3: expected 3 rows after limit, got {len(f3_services)}"
assert f3_values == f4_values[:3], f"F3 values {f3_values} do not match F4[:3] values {f4_values[:3]}"
assert set(f3_services) == set(f4_services[:3]), f"F3 services {f3_services} do not match F4[:3] services {f4_services[:3]}"
# Logs per service at 5, 4, 3, 2, and 1 minutes before now. Totals: a 15, b 17, c 5, d 15.
FORMULA_HAVING_COUNTS = {
"service-a": [1, 4, 2, 5, 3],
"service-b": [6, 1, 1, 2, 7],
"service-c": [1, 1, 1, 1, 1],
"service-d": [3, 3, 3, 3, 3],
}
@pytest.mark.parametrize(
"query_having,formula_having,expected_services",
[
pytest.param("count() > 4 AND count() < 16", "F1 > 4 AND F1 < 16", {"service-a", "service-b"}, id="and_range"),
pytest.param("count() IN (1, 5, 17) OR count() > 15", "__result IN (1, 5, 17) OR __result > 15", {"service-a", "service-b", "service-c"}, id="in_or"),
pytest.param("NOT (count() * 2 - 1 > 9)", "NOT (F1 * 2 - 1 > 9)", {"service-a", "service-b", "service-c", "service-d"}, id="not_arithmetic"),
],
)
def test_logs_formula_having_time_series_matches_query_having(
signoz: types.SigNoz,
create_user_admin: None, # pylint: disable=unused-argument
get_token: Callable[[str, str], str],
insert_logs: Callable[[list[Logs]], None],
query_having: str,
formula_having: str,
expected_services: set[str],
) -> None:
"""
The querier computes formulas in Go, so a formula having runs in Go. For the
formula `A`, it must keep the same points that SQL HAVING keeps on query A.
"""
now = datetime.now(tz=UTC).replace(second=0, microsecond=0)
insert_logs([Logs(timestamp=now - timedelta(minutes=5 - minute), resources={"service.name": service}, body=f"{service} log {minute}-{i}") for service, counts in FORMULA_HAVING_COUNTS.items() for minute, count in enumerate(counts) for i in range(count)])
token = get_token(USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD)
group_by = [build_group_by_field("service.name", "string", "resource")]
response = make_query_request(
signoz,
token,
start_ms=int((now - timedelta(minutes=15)).timestamp() * 1000),
end_ms=int(now.timestamp() * 1000),
request_type="time_series",
queries=[
build_scalar_query(name="A", signal="logs", aggregations=[build_aggregation("count()")], group_by=group_by, disabled=True),
build_scalar_query(name="B", signal="logs", aggregations=[build_aggregation("count()")], group_by=group_by, having_expression=query_having),
build_formula_query("F1", "A", having_expression=formula_having),
],
)
assert response.status_code == HTTPStatus.OK, response.text
query_series = index_series_by_label(get_all_series(response.json(), "B"), "service.name")
formula_series = index_series_by_label(get_all_series(response.json(), "F1"), "service.name")
assert set(formula_series) == expected_services
assert set(query_series) == expected_services
for service in expected_services:
query_points = sorted((value["timestamp"], value["value"]) for value in query_series[service]["values"])
formula_points = sorted((value["timestamp"], value["value"]) for value in formula_series[service]["values"])
assert formula_points == query_points, service
@pytest.mark.parametrize(
"query_having,formula_having,expected_services",
[
pytest.param("count() > 4 AND count() < 16", "F1 > 4 AND F1 < 16", {"service-a", "service-c", "service-d"}, id="and_range"),
pytest.param("count() IN (1, 5, 17) OR count() > 15", "__result IN (1, 5, 17) OR __result > 15", {"service-b", "service-c"}, id="in_or"),
pytest.param("NOT (count() * 2 - 1 > 9)", "NOT (F1 * 2 - 1 > 9)", {"service-c"}, id="not_arithmetic"),
],
)
def test_logs_formula_having_scalar_matches_query_having(
signoz: types.SigNoz,
create_user_admin: None, # pylint: disable=unused-argument
get_token: Callable[[str, str], str],
insert_logs: Callable[[list[Logs]], None],
query_having: str,
formula_having: str,
expected_services: set[str],
) -> None:
"""
For the formula `A` in a scalar request, the formula having must keep the
same rows that SQL HAVING keeps on query A.
"""
now = datetime.now(tz=UTC).replace(second=0, microsecond=0)
insert_logs([Logs(timestamp=now - timedelta(minutes=5 - minute), resources={"service.name": service}, body=f"{service} log {minute}-{i}") for service, counts in FORMULA_HAVING_COUNTS.items() for minute, count in enumerate(counts) for i in range(count)])
token = get_token(USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD)
group_by = [build_group_by_field("service.name", "string", "resource")]
response = make_query_request(
signoz,
token,
start_ms=int((now - timedelta(minutes=15)).timestamp() * 1000),
end_ms=int(now.timestamp() * 1000),
request_type="scalar",
queries=[
build_scalar_query(name="A", signal="logs", aggregations=[build_aggregation("count()")], group_by=group_by, disabled=True),
build_scalar_query(name="B", signal="logs", aggregations=[build_aggregation("count()")], group_by=group_by, having_expression=query_having),
build_formula_query("F1", "A", having_expression=formula_having),
],
)
assert response.status_code == HTTPStatus.OK, response.text
results = response.json()["data"]["data"]["results"]
values_by_query = {}
for name in ("B", "F1"):
result = find_named_result(results, name)
assert result is not None, name
service_column = next(i for i, column in enumerate(result["columns"]) if column["name"] == "service.name")
value_column = next(i for i, column in enumerate(result["columns"]) if column["columnType"] == "aggregation")
values_by_query[name] = {row[service_column]: row[value_column] for row in result["data"]}
assert set(values_by_query["F1"]) == expected_services
assert values_by_query["F1"] == values_by_query["B"]
def test_logs_formula_having_invalid_reference(
signoz: types.SigNoz,
create_user_admin: None, # pylint: disable=unused-argument
get_token: Callable[[str, str], str],
) -> None:
"""A formula having can reference only the formula value, so `count()` is a bad request."""
now = datetime.now(tz=UTC).replace(second=0, microsecond=0)
token = get_token(USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD)
response = make_query_request(
signoz,
token,
start_ms=int((now - timedelta(minutes=15)).timestamp() * 1000),
end_ms=int(now.timestamp() * 1000),
request_type="time_series",
queries=[
build_scalar_query(name="A", signal="logs", aggregations=[build_aggregation("count()")], disabled=True),
build_formula_query("F1", "A", having_expression="count() > 0"),
],
)
assert response.status_code == HTTPStatus.BAD_REQUEST, response.text
assert get_error_message(response.json()) == "Invalid references in `Having` expression: [count]"