[pull] main from SigNoz:main - #945
Merged
Merged
Conversation
<!--A few plain bullets saying what changed and why, for a reviewer skimming it - not a wall of text, not a restatement of the diff, not generated boilerplate.--> #### Description Bumping cloud integration agent version from v0.0.13 to v0.0.14 #### Contributes to SigNoz/platform-pod#3038
#### Description - Update the sample span JSON in the LLM Observability attribute-mapping Test tab to use OpenInference-style attribute keys (`llm.model_name`, `llm.provider`, `llm.token_count.*`, `input.value`, `output.value`) instead of the earlier mix of `gen_ai.*` and placeholder `my_company.*` keys. - The sample is what users see first when trying out attribute mapping, so it should reflect the attribute shape they'll actually be mapping from. #### Issues closed by this PR Closes - https://github.com/orgs/SigNoz/projects/39/views/20?pane=issue&itemId=238442542&issue=SigNoz%7Cengineering-pod%7C5997 #### Screenshots / Screen Recordings <img width="911" height="572" alt="image" src="https://github.com/user-attachments/assets/88df20e9-0131-415c-9770-67ea8196075a" /> #### Additional Information - Constant-only change (`SAMPLE_SPAN_JSON` in `spanInputStorage.ts`); no parsing or mapping logic touched. - Worth a sanity check that the new keys line up with the attribute names the mapping UI is expected to suggest. --------- Co-authored-by: Gaurav Tewari <tewarig@users.noreply.github.com>
#### Description **What was broken:** on a stack using the new JSON log body, any query asking for a **table** (`scalar`) or a **graph** (`time_series`) failed with HTTP 500 if the result contained a JSON column. The logs list view worked fine, which is why this went unnoticed. The smallest way to hit it is a raw ClickHouse panel running `select * from signoz_logs.logs_v2`. **Why it happened:** we ask ClickHouse to send JSON columns as plain text, but the driver reports that such a column needs a different Go type — so the reader prepared the wrong kind of container and the read failed. The driver only reports the correct type *after* the first row has been read, which is too late for code that sets up its containers up front. The original JSON work patched around this inside the logs-list reader only; the connection setting that causes it is global, so the other two readers stayed broken. **The fix:** correct the reported type once, at the connection that sets that option, so every reader gets a container that works and receives a normal map. Concretely: - `pkg/querier` no longer needs its own workaround — the three readers are back to ordinary code. - The older v3/v4 read paths had the same bug and are fixed without any changes of their own. - A JSON path value such as `body_v2.level` now comes back as `"error"` or `7` instead of a driver wrapper object. - Grouping a graph by the whole JSON body used to collapse every group into a single unlabelled line; each document now labels its own series. #### Issues closed by this PR Fixes SigNoz/engineering-pod#5911 #### Additional Information Verified end to end against a local stack with 1,000,000 log rows and 200,002 distinct `trace_id`s: | query | before | after | | --- | --- | --- | | table query over a JSON column | 500 | 200, body returned as an object | | graph grouped by the JSON body | 500 | 200, 22 series, one per document | | graph grouped by `trace_id` (200k groups) | 200 | 200, unchanged | A follow-up PR stacked on this one reworks how the graph reader classifies columns — fixing boolean and small-integer columns in raw SQL panels and cutting the reader's allocations. Known gaps, unchanged from `main` and out of scope here: - Waterfall and flamegraph read rows into structs, which this fix does not cover. Moving span attributes to JSON will need the same type on those fields, and one helper there fails silently rather than erroring. - Dashboard variable queries no longer crash on a JSON column but still reject it as an unsupported value type. - A `Map(String, JSON)` column **panics inside the driver**, which can take the process down. Confirmed still unfixed on `clickhouse-go` main, and not yet reported upstream.
#### Description Adding support for system dashboards. * as of now updates are only through new versions in the file. * user cannot update the dashboard * For now kept the dashboard content empty and will raise it separately. Closes SigNoz/engineering-pod#4501
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )