(do not review) staging test for trace funnels after merge

[shivanshuraj1333]

Important

Enhance trace funnel analytics with new ClickHouse queries and API endpoints, and enable funnel features in all environments.

• Trace Funnel Analytics:
  • Add new ClickHouse queries in clickhouse_queries.go for funnel validation, overview, step overview, count, and slow/error traces.
  • Implement query.go to build and execute these queries.
• API Endpoints:
  • Add endpoints in http_handler.go for validating traces, funnel analytics, step analytics, slow traces, and error traces.
  • Support both funnel ID and payload-based requests.
• Frontend Changes:
  • Remove NODE_ENV checks in Success.tsx and index.tsx, enabling funnel features in all environments.

^{This description was created by for 71cadd5. You can customize this summary. It will automatically update as commits are pushed.}

[request-info]

We would appreciate it if you could provide us with more info about this issue/pr!

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to 836e0fb in 2 minutes and 42 seconds. Click for details.

• Reviewed 1859 lines of code in 11 files
• Skipped 0 files when reviewing.
• Skipped posting 7 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. pkg/query-service/app/traces/v4/query_builder.go:16

• Draft comment:
  Ensure that operator mappings, especially for regex and exists, use properly sanitized inputs to prevent SQL injection.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

2. pkg/query-service/app/traces/v4/query_builder.go:78

• Draft comment:
  The existsSubQueryForFixedColumn function uses an empty string check as a hack; consider refactoring to a more explicit and clear implementation.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. pkg/query-service/app/traces/v4/query_builder.go:90

• Draft comment:
  Refactor the similar logic in BuildTracesFilterQuery and BuildTracesFilter to reduce duplication and ease maintenance.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

4. pkg/query-service/app/traces/v4/query_builder.go:300

• Draft comment:
  Extract the magic number 1800 used in bucketStart calculation into a named constant to improve code clarity.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

5. pkg/query-service/app/traces/v4/query_builder.go:52

• Draft comment:
  Verify that utils.GetClickhouseColumnNameV2 properly escapes and sanitizes column names when wrapping them in backticks.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

6. pkg/query-service/app/traces/v4/query_builder.go:297

• Draft comment:
  Ensure that all dynamic string concatenations in buildTracesQuery are properly sanitized to avoid any SQL injection risks.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/FunnelStep.tsx:200

• Draft comment:
  There appears to be a stray comment terminator (*/) appended to the closing </div> tag. If this is unintentional, please remove the extra */.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

Workflow ID: wflow_dB0Or4clNa8W2eBu

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed f142eef in 2 minutes and 7 seconds. Click for details.

• Reviewed 118 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 8 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. pkg/modules/tracefunnel/clickhouse_queries.go:175

• Draft comment:
  Changed aggregation functions now use COUNT(DISTINCT ...) with CASE expressions. This ensures unique trace IDs are counted rather than summing multiple rows. Please confirm that deduplicating spans (and error counts) is the intended behavior and that any potential performance impact is acceptable.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 30% <= threshold 50% The comment is asking the PR author to confirm their intention regarding the use of COUNT(DISTINCT ...) with CASE expressions. This violates the rule against asking for confirmation of intention. However, it does point out a potential performance impact, which could be useful. Overall, the comment is more about confirming intention than providing a specific suggestion or pointing out a clear issue.

2. pkg/modules/tracefunnel/clickhouse_queries.go:265

• Draft comment:
  Similar update here for the three‐step funnel totals: using DISTINCT trace_id and CASE expressions. Verify that aggregating unique trace IDs (instead of summing raw error flags) matches the intended business logic across funnel steps.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to verify if the change matches the intended business logic, which violates the rule against asking for confirmation of intention. It doesn't provide a specific suggestion or point out a clear issue with the code.

3. pkg/modules/tracefunnel/clickhouse_queries.go:334

• Draft comment:
  In this two‐step funnel count query, COUNT(DISTINCT ...) replaces count() and countIf. Ensure that counting unique trace IDs for both total and errored spans reflects the desired metric, and note that using DISTINCT may yield different numbers if a trace has multiple qualifying spans.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

4. pkg/modules/tracefunnel/clickhouse_queries.go:400

• Draft comment:
  The three‐step funnel count query now consistently uses DISTINCT counting for both total spans and errored spans. Confirm that switching from SUM to DISTINCT counts doesn’t unintentionally mask multiple error occurrences if those were important.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

5. pkg/modules/tracefunnel/clickhouse_queries.go:604

• Draft comment:
  Aggregation in this query now uses DISTINCT trace_id to count total spans and errors, ensuring deduplication. Please validate that this behavior meets the desired reporting metrics and consider any performance impacts when processing large datasets.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to validate the behavior and consider performance impacts, which violates the rule against asking the author to confirm their intention or ensure behavior. It doesn't provide a specific suggestion or point out a clear issue with the code.

6. pkg/modules/tracefunnel/clickhouse_queries.go:721

• Draft comment:
  Here the error aggregations have been changed from SUM(s_error) to COUNT(DISTINCT CASE WHEN s_error = 1 THEN trace_id END). Ensure that this accurately captures error traces, especially if a trace might record multiple errors.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the author to ensure that the change accurately captures error traces. This falls under the rule of not asking the author to ensure the behavior is intended. Therefore, this comment should be removed.

7. pkg/modules/tracefunnel/clickhouse_queries.go:740

• Draft comment:
  The final changes use COUNT(DISTINCT CASE WHEN ...) for total and error spans in this funnel section. Verify that deduplicating trace IDs here aligns with your tracking requirements, and review any potential performance trade‑offs with large-scale data.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

8. pkg/modules/tracefunnel/clickhouse_queries.go:748

• Draft comment:
  The use of the placeholder %[19]s in the call to quantileIf looks unusual. Please confirm if this is intentional or if it should be replaced with a specific numeric value (e.g., 0.19) to avoid unexpected behavior.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_72WDKM3tYQoF3jYc

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 0350f4a in 1 minute and 59 seconds. Click for details.

• Reviewed 13 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 7 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. pkg/query-service/app/http_handler.go:5048

• Draft comment:
  In the getQueueOverview handler, the call to GetListResultV3 does not check for an error before sending a response. This might mask underlying issues. Please add proper error handling before calling aH.Respond.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

2. pkg/query-service/app/http_handler.go:2194

• Draft comment:
  In the Kafka onboarding functions (e.g. onboardProducers and onboardConsumers), the code uses a type assertion with *value.(*uint8) without prior type checking. This could panic if the data is not of the expected type. Consider verifying the type with a safe type assertion before dereferencing.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. pkg/query-service/app/http_handler.go:4776

• Draft comment:
  In liveTailLogsV2 the request body is replaced with a reader built from the URL query parameter 'q'. This reuse of r.Body may lead to unexpected behavior or confusion. Consider either handling the query parameter directly or clearly documenting this behavior.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

4. pkg/query-service/app/http_handler.go:4157

• Draft comment:
  In ListLogsPipelinesHandler, context.Background() is used when calling GetLatestVersion instead of the request's context. Using r.Context() would better support cancellation and timeouts, so consider replacing context.Background() with r.Context().
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

5. pkg/query-service/app/http_handler.go:4581

• Draft comment:
  The sendQueryResultEvents helper compiles regex patterns on each call. This may hurt performance under heavy load. It would be more efficient to precompile these regular expressions once (for example, as package‑level variables) and reuse them.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

6. pkg/query-service/app/http_handler.go:4100

• Draft comment:
  There is inconsistent handling of the request body across different handlers. Some handlers call r.Body.Close() while others do not. To prevent potential resource leaks, ensure that r.Body is consistently closed in all handlers after reading.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. pkg/query-service/app/http_handler.go:1

• Draft comment:
  The http_handler.go file is very large and contains a wide variety of endpoint handlers. This can make maintenance and debugging difficult. Consider refactoring the file into smaller, focused modules (for example, grouping endpoints by domain such as metrics, logs, traces, integrations, etc.).
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_g3TLK8kc9Vjdp11L

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 0c454d1 in 50 seconds. Click for details.

• Reviewed 39 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. pkg/modules/tracefunnel/query.go:17

• Draft comment:
  Consider using a case‐insensitive check when determining if the clause already starts with "AND". This would handle cases where the clause might begin with "and" instead of "AND".
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

2. pkg/query-service/app/traces/v3/query_builder.go:196

• Draft comment:
  Consider a case‐insensitive check for the prefix 'AND' in the query string to ensure consistency even if the clause is lowercase.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_I4cGc2VzVPJNLvxD

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed acdb2da in 1 minute and 51 seconds. Click for details.

• Reviewed 55 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 4 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. pkg/modules/tracefunnel/clickhouse_queries.go:628

• Draft comment:
  Changing from IN clauses with separate AND conditions to an OR block alters the row selection. Verify that limiting rows to exactly the matching (serviceName, name) pairs is the intended behavior.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

2. pkg/modules/tracefunnel/clickhouse_queries.go:703

• Draft comment:
  For the three‐step funnel query, the filtering now uses an OR block (with explicit equality for each step) instead of IN clauses plus separate conditions. Confirm this change meets the funnel logic requirements.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. pkg/modules/tracefunnel/clickhouse_queries.go:722

• Draft comment:
  The inline comment ‘-- eligible only’ was removed for the total span count. Ensure this removal does not reduce clarity about the metric definition.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

4. pkg/modules/tracefunnel/clickhouse_queries.go:628

• Draft comment:
  Using explicit boolean comparison (‘has_error = true’) increases clarity compared to relying on implicit truthiness. Confirm that this aligns correctly with the data type for 'has_error' in ClickHouse.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_bFk98hcbHbG0wzkc

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 9bdc8c7 in 1 minute and 31 seconds. Click for details.

• Reviewed 55 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 3 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. pkg/modules/tracefunnel/clickhouse_queries.go:628

• Draft comment:
  Refactored two‐step funnel filtering: verify that combining conditions with OR (explicitly checking serviceName and span name along with error flags) preserves the intended semantics compared to the prior IN/AND approach.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the PR author to verify the semantics of a refactored code, which violates the rule against asking for confirmation of intention or behavior. It does not provide a specific code suggestion or ask for a specific test to be written.

2. pkg/modules/tracefunnel/clickhouse_queries.go:703

• Draft comment:
  For the three‐step funnel query, confirm that the OR grouping of each step’s conditions (with explicit service and span name checks and error logic) correctly replicates the desired filtering behavior.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. pkg/modules/tracefunnel/clickhouse_queries.go:722

• Draft comment:
  The removal of the "eligible only" comment from the aggregated count column is noted; ensure that this omission is intentional and that any needed documentation about the eligibility logic is maintained.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_wMlIGCUhahiKGjK8

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 71cadd5 in 1 minute and 39 seconds. Click for details.

• Reviewed 113 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 4 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/container/TraceWaterfall/TraceWaterfallStates/Success/Success.tsx:150

• Draft comment:
  Removed dev-only check for funnel button rendering. Confirm that funnel functionality should be enabled in all environments.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

2. frontend/src/container/TraceWaterfall/TraceWaterfallStates/Success/Success.tsx:475

• Draft comment:
  Removed environment check for rendering AddSpanToFunnelModal. Verify that the funnel modal is intended to be active in non-development environments.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the PR author to verify their intention regarding the removal of an environment check. It doesn't provide a specific suggestion or point out a clear issue, which violates the rule against asking the author to confirm their intention.

3. frontend/src/pages/TraceDetailV2/index.tsx:67

• Draft comment:
  The dev-only condition for including the Funnels tab has been removed. Confirm that the Funnels tab should now always be visible.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

4. frontend/src/pages/TracesModulePage/TracesModulePage.tsx:16

• Draft comment:
  Funnel route is now always included instead of being gated by the dev environment. Ensure this change aligns with the intended release strategy.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

Workflow ID: wflow_zadCNTy0MKAYtogu

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}