feat: api for trace materialization

[nityanandagohain]

feat: api for trace materialization

Fixes #6584

Changes

• remane logfield to field.
• added /api/v2/traces/fields instead of /api/v1/traces/fields as it was colliding with search traces /api/v1/traces/{traceId}

---

Important

Introduces new API endpoints for trace materialization, renames LogField to Field, and updates validation and interfaces for enhanced trace handling.

• API Changes:
  • Added new endpoints /api/v2/traces/fields for GET and POST requests in http_handler.go.
  • Implemented traceFields() and updateTraceField() in APIHandler.
• Data Structure Changes:
  • Renamed LogField to Field in response.go and updated related functions.
  • Updated GetFieldsResponse to use Field instead of LogField.
• Validation:
  • Added ValidateUpdateFieldPayloadV2() in validator.go for new trace field validation.
• Miscellaneous:
  • Updated Reader interface in interface.go to include GetTraceFields() and UpdateTraceField().

^{This description was created by for 2ca724a. It will automatically update as commits are pushed.}

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 01ed3c2 in 1 minute and 46 seconds

More details

• Looked at 433 lines of code in 8 files
• Skipped 0 files when reviewing.
• Skipped posting 11 drafted comments based on config settings.

1. pkg/query-service/app/clickhouseReader/reader.go:2696

• Draft comment:
  The function removeUnderscoreDuplicateFields has been updated to use model.Field instead of model.LogField. This change is consistent with the rest of the code, where model.Field is used instead of model.LogField. This change is correct and aligns with the PR's intent to rename LogField to Field.
• Reason this comment was not posted:
  Confidence changes required: 0%
  The function removeUnderscoreDuplicateFields has been updated to use model.Field instead of model.LogField. This change is consistent with the rest of the code, where model.Field is used instead of model.LogField. This change is correct and aligns with the PR's intent to rename LogField to Field.

2. pkg/query-service/app/clickhouseReader/reader.go:2719

• Draft comment:
  The function extractSelectedAndInterestingFields has been updated to use model.Field instead of model.LogField. This change is consistent with the rest of the code, where model.Field is used instead of model.LogField. This change is correct and aligns with the PR's intent to rename LogField to Field.
• Reason this comment was not posted:
  Confidence changes required: 0%
  The function extractSelectedAndInterestingFields has been updated to use model.Field instead of model.LogField. This change is consistent with the rest of the code, where model.Field is used instead of model.LogField. This change is correct and aligns with the PR's intent to rename LogField to Field.

3. pkg/query-service/app/clickhouseReader/reader.go:2947

• Draft comment:
  The function GetTraceFields is introduced to fetch trace fields. It correctly initializes the response with selected and interesting fields. The function also queries the database for attribute keys and processes them. The use of defer rows.Close() ensures that the database rows are closed properly, preventing resource leaks. The function seems to be implemented correctly.
• Reason this comment was not posted:
  Confidence changes required: 0%
  The function GetTraceFields is introduced to fetch trace fields. It correctly initializes the response with selected and interesting fields. The function also queries the database for attribute keys and processes them. The use of defer rows.Close() ensures that the database rows are closed properly, preventing resource leaks. The function seems to be implemented correctly.

4. pkg/query-service/app/clickhouseReader/reader.go:2948

• Draft comment:
  The function UpdateTraceField is introduced to update trace fields. It correctly checks if the field is selected and constructs the column name and data type for the materialized column. The function also handles the creation of the index if the data type is not boolean. The function seems to be implemented correctly.
• Reason this comment was not posted:
  Confidence changes required: 0%
  The function UpdateTraceField is introduced to update trace fields. It correctly checks if the field is selected and constructs the column name and data type for the materialized column. The function also handles the creation of the index if the data type is not boolean. The function seems to be implemented correctly.

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

• Draft comment:
  The RegisterRoutes function has been updated to include new routes for trace fields. The routes /api/v2/traces/fields for GET and POST methods are correctly added with appropriate access levels. This change aligns with the PR's intent to add API endpoints for trace materialization.
• Reason this comment was not posted:
  Confidence changes required: 0%
  The RegisterRoutes function has been updated to include new routes for trace fields. The routes /api/v2/traces/fields for GET and POST methods are correctly added with appropriate access levels. This change aligns with the PR's intent to add API endpoints for trace materialization.

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

• Draft comment:
  The traceFields and updateTraceField functions are introduced to handle the new API endpoints for trace fields. They correctly interact with the reader to fetch and update trace fields, respectively. The error handling and response writing are consistent with the existing codebase practices.
• Reason this comment was not posted:
  Confidence changes required: 0%
  The traceFields and updateTraceField functions are introduced to handle the new API endpoints for trace fields. They correctly interact with the reader to fetch and update trace fields, respectively. The error handling and response writing are consistent with the existing codebase practices.

7. pkg/query-service/app/logs/parser.go:229

• Draft comment:
  The arrayToMap function has been updated to use model.Field instead of model.LogField. This change is consistent with the rest of the code, where model.Field is used instead of model.LogField. This change is correct and aligns with the PR's intent to rename LogField to Field.
• Reason this comment was not posted:
  Confidence changes required: 0%
  The arrayToMap function has been updated to use model.Field instead of model.LogField. This change is consistent with the rest of the code, where model.Field is used instead of model.LogField. This change is correct and aligns with the PR's intent to rename LogField to Field.

8. pkg/query-service/app/logs/parser.go:253

• Draft comment:
  The replaceFieldInToken function has been updated to use model.Field instead of model.LogField. This change is consistent with the rest of the code, where model.Field is used instead of model.LogField. This change is correct and aligns with the PR's intent to rename LogField to Field.
• Reason this comment was not posted:
  Confidence changes required: 0%
  The replaceFieldInToken function has been updated to use model.Field instead of model.LogField. This change is consistent with the rest of the code, where model.Field is used instead of model.LogField. This change is correct and aligns with the PR's intent to rename LogField to Field.

9. pkg/query-service/app/clickhouseReader/reader.go:2947

• Draft comment:
  Do not add non-ClickHouse related functions to the ClickHouseReader interface. Use the DAO in the telemetry instance to access such information instead.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The function GetTraceFields appears to be related to ClickHouse as it involves querying data from a ClickHouse database. The comment might be suggesting a design pattern or architectural decision, but without more context, it's hard to determine if this function violates that rule.
  I might be missing the broader architectural context or design patterns intended for the ClickHouseReader interface. The comment could be based on a specific design decision that isn't evident from the code alone.
  Without explicit evidence that GetTraceFields is non-ClickHouse related, it's reasonable to assume it is related given its database interactions. The comment might be more about design philosophy than a strict rule.
  The comment seems speculative without clear evidence that GetTraceFields is non-ClickHouse related. It should be removed unless there's explicit evidence of a design rule being violated.

10. pkg/query-service/interfaces/interface.go:115

• Draft comment:
  Do not add non-ClickHouse related functions to the ClickHouseReader interface. Use the DAO in the telemetry instance to access such information instead.
• Reason this comment was not posted:
  Marked as duplicate.

11. pkg/query-service/app/logs/validator.go:43

• Draft comment:
  This is a duplicate of the existing validation function. Consider extending the existing function to handle both logs and traces cases instead of creating a new one.

• function ValidateUpdateFieldPayload (validator.go)

• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The comment suggests that the new function is a duplicate and could be merged with the existing one. However, the new function uses different constants for validation, which suggests it serves a different purpose. The comment may not fully consider the context or reason for the new function's existence.
  The comment might be missing the context that the new function is intended for a different use case, as indicated by the different constants used in the regex pattern. The functions may not be directly mergeable due to these differences.
  The presence of different constants in the regex pattern suggests that the new function is intended for a different context, which might justify its existence as a separate function.
  The comment should be deleted as it does not consider the different contexts indicated by the use of different constants in the regex patterns of the two functions.

Workflow ID: wflow_1iTCw3RU8Mp8HoGc

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on d16f31f in 1 minute and 10 seconds

More details

• Looked at 102 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 2 drafted comments based on config settings.

1. pkg/query-service/app/logparsingpipeline/controller.go:97

• Draft comment:
  The ValidatePipelines function is duplicated. Consider removing the duplicate to avoid redundancy.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The comment seems to be incorrect as there is no evidence of a duplicate ValidatePipelines function in the provided file content. The comment does not point to any specific duplication within the diff or the file.
  It's possible that the duplication exists in another part of the codebase not shown here, but based on the provided context, there is no duplication visible.
  Without evidence of duplication in the provided context, the comment should be considered incorrect and removed.
  The comment about the ValidatePipelines function being duplicated is incorrect based on the provided context and should be deleted.

2. pkg/query-service/app/logparsingpipeline/controller.go:97

• Draft comment:
  Avoid using inline styles. Use external stylesheets or styled components instead. This is also applicable in other similar functions.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_agakEiKwAt3LwEjX

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[srikanthccv]

minor comments

[ellipsis-dev]

👍 Looks good to me! Incremental review on 2ca724a in 3 minutes and 4 seconds

More details

• Looked at 27 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 2 drafted comments based on config settings.

1. pkg/query-service/app/clickhouseReader/reader.go:3094

• Draft comment:
  The index granularity for the minmax index is hardcoded to 1. Consider allowing customization of the granularity to optimize performance for different use cases.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  Let me analyze this carefully:

1. The comment suggests making the granularity configurable instead of hardcoded
2. Looking at the code context, this is about creating a minmax index for numeric trace fields
3. The change removed an existing granularity parameter and hardcoded it to 1
4. The comment is speculative ("Consider...") and doesn't definitively identify a problem
5. The comment suggests a potential optimization but doesn't demonstrate clear evidence that variable granularity is needed
   The comment could be valid - index granularity can affect query performance and storage size. However, without evidence that variable granularity is actually needed, this may be premature optimization.
   The rules state to delete speculative comments that start with "Consider..." and to only keep comments with strong evidence. While granularity customization could be useful, there's no clear evidence presented that the hardcoded value of 1 is problematic.
   The comment should be deleted because it is speculative and lacks strong evidence that variable granularity is needed. It violates the rule about not making speculative suggestions.

2. pkg/query-service/app/clickhouseReader/reader.go:2948

• Draft comment:
  Do not add non-ClickHouse related functions to the ClickHouseReader interface. Use the DAO in the telemetry instance to access such information instead.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_94SohYHsJANQFB5Y

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.