Skip to content

Conversation

@piyushsingariya
Copy link
Contributor

@piyushsingariya piyushsingariya commented Nov 17, 2025

πŸ“„ Summary

These changes includes some basic utilities that will be used in Body JSON Query Builder


βœ… Changes

  • Chore: JSON Body utils

πŸ” Related Issues

Closes #


πŸ“‹ Checklist

  • Dev Review
  • Test cases added (Unit/ Integration / E2E)
  • Manually tested the changes

πŸ‘€ Notes for Reviewers


Note

Introduces body JSON metadata (paths, values, indexes, promotions) and enables body-context JSON querying across logs, with refactored builders/types, new tests, and a collector dependency bump.

  • Body JSON Metadata (new):
    • Add telemetrymetadata: list body JSON paths, values, indexes; detect promoted paths; shared table constants.
  • Query & Condition Building:
    • Support FieldContextBody and body. prefix in filters, functions, and aggregations; remove JsonBodyPrefix plumbed arg.
    • Update logs/traces condition builders and field mappers to new schema type enums and body JSON exists/array handling.
    • Expose BODY_JSON_QUERY_ENABLED flag; tweak PreparedWhereClause fields; improve LIKE warnings.
  • Types:
    • Add JSON data type model/mappings (JSONDataType, indexes) and extend FieldDataType; add FieldContextBody and BodyJSONStringSearchPrefix.
  • Logs Query Pipeline:
    • Enhance statement builder to pass body JSON key resolution; keep full-text and resource filter integration.
  • Tests:
    • Extensive updates/additions for body JSON queries, warnings, and condition building.
  • Errors/Deps:
    • Add timeout error helpers; bump signoz-otel-collector to v0.129.10-rc.9.

Written by Cursor Bugbot for commit 1bb60bb. This will update automatically on new commits. Configure here.

@cursor
Copy link

cursor bot commented Nov 17, 2025

You have run out of free Bugbot PR reviews for this billing cycle. This will reset on December 3.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

@github-actions github-actions bot added the enhancement New feature or request label Nov 17, 2025
Copy link
Member

@nityanandagohain nityanandagohain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please have a look at the query, it seems to be failing.


Also this feels like the body_json_keys should be a part of metadata package instead and should also use FieldKeySelector struct . @srikanthccv what are your thoughts on this


Also please update the PR name and description as this PR is more than utils and it has all the main metadata functions required for JSON querying to work.

@srikanthccv
Copy link
Member

Also this feels like the body_json_keys should be a part of metadata package instead and should also use FieldKeySelector struct

I checked the main PR to get some prior context and I agree. What do you think about keeping JSONQueryBuilder stateless; where it takes the relevant context from inputs and returns the condition? As I understand it, it's currently using the ClickHouse conn to build intelligent conditions based on: 1) indexing and 2) promotion.

How about we make telemetrytypes.TelemetryFieldKey/FieldKeySelector etc.. aware of this context and have the metadata package annotate the keys returned from GetKeys/GetKeysMulti with the JSON context? That way, JSONQueryBuilder would still have all the metadata needed to prepare the correct and performant query.

The way I view the work you're doing is that it's laying the foundation for all JSON work going forward. As such, it's better to keep JSONQueryBuilder stateless and independent of logs (at a high level) and ensure it remains pluggable if we want to introduce JSON support for span attributes. Let me know what you think.

@piyushsingariya
Copy link
Contributor Author

piyushsingariya commented Nov 19, 2025

What do you think about keeping JSONQueryBuilder stateless;

Can not be stateless

How about we make telemetrytypes.TelemetryFieldKey/FieldKeySelector etc.. aware of this context and have the metadata package annotate the keys returned from GetKeys/GetKeysMulti with the JSON context? That way, JSONQueryBuilder would still have all the metadata needed to prepare the correct and performant query.

The way I view the work you're doing is that it's laying the foundation for all JSON work going forward. As such, it's better to keep JSONQueryBuilder stateless and independent of logs (at a high level) and ensure it remains pluggable if we want to introduce JSON support for span attributes. Let me know what you think.

Will do this later, right now JSONQueryBuilder is acting as a separate branch code that's flag control, it doesn't fit into the current rhythm of QB being stateless, QB would need large changes that I should not touch right now, which I had evaluated at the very start, hence built it in such way

My plan is to first overhaul Logs QB, and then based on chaos there afterwards I'll might touch the main QB

@srikanthccv
Copy link
Member

Please help me understand why can't it be stateless.

@piyushsingariya
Copy link
Contributor Author

piyushsingariya commented Nov 19, 2025

Please help me understand why can't it be stateless.

It needs to know types for each paths existing in warehouse to form the actual queries.

e.g. Input value is string but the operator is Contains, existing type is Array of String, the operation would be to look into array of string contains the value give.

This type of decision making is required in-flight.

@piyushsingariya piyushsingariya changed the title feat(JSON): JSON Body Utils feat(JSON): JSON Body Metadata Nov 19, 2025
@srikanthccv
Copy link
Member

That's what we do in the condition builder + field mapper already. Prepare the right query based on the FieldContext + FieldDataType + Materialised info of the each that it gets. Can you please give an example why can't be the same done that it requires a state management.

@piyushsingariya
Copy link
Contributor Author

Can you please give an example why can't be the same done that it requires a state management.

I just iterated over one above. Will give one more example.

something.something1[].name

something1 here could be a simple Array(JSON) or an Array(Dynamic) based on the state it will form a query to just look into Array(JSON) if it's Array(Dynamic) it will then filter JSON array out of Dynamic and then dive in for name field, if both it will look both cases

@srikanthccv
Copy link
Member

I get that, but what I am not able to follow is why there is a need to maintain a state. The relevant context would be available as part of the TelemetryFieldKey, and the condition can be built correctly based on the known context.

@piyushsingariya piyushsingariya added safe-to-integrate Run integration tests staging:civil-eagle Staging for Piyush and removed safe-to-integrate Run integration tests staging:civil-eagle Staging for Piyush labels Dec 3, 2025
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is being reviewed by Cursor Bugbot

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

@nityanandagohain nityanandagohain merged commit e66bfe5 into main Dec 9, 2025
13 checks passed
@nityanandagohain nityanandagohain deleted the body-json-keys branch December 9, 2025 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request safe-to-integrate Run integration tests staging:civil-eagle Staging for Piyush

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants