Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trace static fields and structs for response. #6469

Merged
merged 4 commits into from
Nov 19, 2024

Conversation

nityanandagohain
Copy link
Member

@nityanandagohain nityanandagohain commented Nov 18, 2024

  • Has new static fields of traces (used for suggestions)
  • Has deprecated static fields of traces ( use to support old names)
  • Static field traces is a union of the above
  • The response structs will be used in trace detail page.

ref for static fields https://github.com/SigNoz/signoz-otel-collector/blob/main/cmd/signozschemamigrator/schema_migrator/traces_migrations.go

FOR #5713


Important

Adds new static fields for traces, deprecates old ones, and introduces new response structs for trace details.

  • Static Fields:
    • Introduces NewStaticFieldsTraces and DeprecatedStaticFieldsTraces in constants.go.
    • Combines both into StaticFieldsTraces using init() function.
  • Response Structs:
    • Adds SpanItemV2 and TraceSummary structs in trace.go for trace detail responses.

This description was created by Ellipsis for ce7b509. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 6fce317 in 1 minute and 13 seconds

More details
  • Looked at 413 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 drafted comments based on config settings.
1. pkg/query-service/model/response.go:272
  • Draft comment:
    The SearchSpanResponseItemV2 struct is introduced but not used anywhere in the codebase. Ensure that this struct is utilized or planned for future use to avoid dead code.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR introduces a new struct SearchSpanResponseItemV2 but does not provide any context or usage for it. It is important to ensure that new code is utilized or at least planned for future use.
2. pkg/query-service/model/response.go:291
  • Draft comment:
    The TraceSummary struct is introduced but not used anywhere in the codebase. Ensure that this struct is utilized or planned for future use to avoid dead code.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR introduces a new struct TraceSummary but does not provide any context or usage for it. It is important to ensure that new code is utilized or at least planned for future use.
3. pkg/query-service/constants/constants.go:451
  • Draft comment:
    The NewStaticFieldsTraces map is introduced but not used anywhere in the codebase. Ensure that this map is utilized or planned for future use to avoid dead code.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR introduces a new map NewStaticFieldsTraces but does not provide any context or usage for it. It is important to ensure that new code is utilized or at least planned for future use.
4. pkg/query-service/constants/constants.go:569
  • Draft comment:
    The DeprecatedStaticFieldsTraces map is introduced but not used anywhere in the codebase. Ensure that this map is utilized or planned for future use to avoid dead code.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR introduces a new map DeprecatedStaticFieldsTraces but does not provide any context or usage for it. It is important to ensure that new code is utilized or at least planned for future use.
5. pkg/query-service/constants/constants.go:1
  • Draft comment:
    Avoid using the component/index.tsx file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. This applies to the entire file.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is about a TypeScript file structure, which is not applicable to this Go file. The comment does not address any changes made in the diff and seems misplaced.
    I might be missing some context about the project structure, but based on the file content, the comment seems irrelevant.
    The file content and the language used (Go) strongly indicate that the comment is misplaced.
    The comment should be deleted as it is not relevant to the changes made in this Go file.

Workflow ID: wflow_77iZeYRzSh8Xypwb


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

@eKuG
Copy link
Contributor

eKuG commented Nov 18, 2024

So are we moving from camelCase to snakeCase? Just wanted to know for context. I see some changes in the previous PR as well @nityanandagohain

@nityanandagohain nityanandagohain merged commit 57c2326 into develop Nov 19, 2024
18 checks passed
@nityanandagohain nityanandagohain deleted the feat/const-and-struct branch November 19, 2024 09:08
YounixM pushed a commit that referenced this pull request Nov 20, 2024
* fix: update static fields and add response structs

* fix: update ch names

* fix: move models to it's own file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants