feat: add the new version package#7411
Conversation
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 5d92447 in 2 minutes and 34 seconds
More details
- Looked at
852lines of code in18files - Skipped
0files when reviewing. - Skipped posting
10drafted comments based on config settings.
1. pkg/query-service/telemetry/telemetry.go:99
- Draft comment:
Avoid hardcoding API keys (e.g. the key on line 99). Use environment variables or secure config to inject sensitive credentials. - 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/version/version.go:10
- Draft comment:
Using an identifier named 'time' as a variable can be confusing given the 'time' package import. Consider renaming it (e.g., to 'buildTime'). - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
While renaming tobuildTimecould be slightly more descriptive, the current code already avoids confusion by aliasing the time package asgotime. The variable's purpose is clear from context (build info struct) and comments. The suggestion seems more like a minor preference than a significant improvement.
The variable nametimeis quite generic and could be more specific. The code would be slightly more self-documenting withbuildTime.
The potential confusion is already mitigated by the package aliasgotime, clear comments, and the context of being in a build info struct. The benefit of renaming is minimal.
Delete this comment. While the suggestion isn't wrong, it's not important enough to warrant a comment given that the code is already clear and unambiguous.
3. pkg/signoz/config.go:115
- Draft comment:
Consider using a logger (instead of fmt.Println) for deprecation warnings to ensure consistent logging across environments. - 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/version/version.go:105
- Draft comment:
The ASCII banner is printed via fmt.Println. Consider using a logging mechanism or allowing configuration to disable banner output in production. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
The code already has configuration control via cfg.Banner.Enabled. Using fmt.Println is reasonable for a banner that's meant to be visible output, not a log message. Logging mechanisms are typically for diagnostic/error information, while this is intentional user-facing output. The comment's concern about production output is already addressed by the config flag.
Maybe there are cases where redirecting the banner output to different writers would be useful? The current implementation is coupled to stdout.
While writer flexibility could be nice, the current implementation is simple and sufficient for a version banner. The key concern about production control is already handled by the config.
Delete the comment. The code already has configuration control, and fmt.Println is appropriate for intentional user-facing output like a version banner.
5. pkg/query-service/telemetry/telemetry.go:99
- Draft comment:
Avoid hardcoding the API key ('9kRrJ7oPCGPEJLF6QjMPLt5bljFhRQBr'); instead load it from a secure environment variable. - 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/telemetry/telemetry.go:530
- Draft comment:
The getOutboundIP function calls an external service each time; consider caching the result to avoid performance delays or reliance on network connectivity. - 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/signoz/signoz.go:112
- Draft comment:
Ensure that sensitive config values (like database DSNs) are not logged accidentally. The initialization logs config details; verify these do not leak secrets. - 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.
8. .github/workflows/testing-deployment.yaml:49
- Draft comment:
Typo found: 'scenerio' should be changed to 'scenario' for clarity. - Reason this comment was not posted:
Comment was on unchanged code.
9. conf/example.yaml:121
- Draft comment:
Typo found: 'exahustive' should be corrected to 'exhaustive'. - 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.
10. pkg/query-service/telemetry/telemetry.go:585
- Draft comment:
Typo in comment: consider changing 'Updating a groups properties' to 'Updating a group's properties' for 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.
Workflow ID: wflow_l2cC9tKuVkKkGxuf
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 29c855b in 1 minute and 28 seconds
More details
- Looked at
77lines of code in2files - Skipped
0files when reviewing. - Skipped posting
5drafted comments based on config settings.
1. .github/workflows/goci.yaml:38
- Draft comment:
Consider extracting the repeated 'if' condition block into a YAML anchor to avoid duplication. - 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. .github/workflows/goci.yaml:50
- Draft comment:
Consider splitting the combined 'make' commands into separate steps for better failure isolation. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
While splitting steps could provide clearer failure messages in GitHub Actions, this is a relatively minor workflow optimization. The current setup is perfectly functional and follows common practices. The failure messages would still indicate which command failed. This seems more like a personal preference than a critical issue requiring change.
The suggestion could improve workflow debugging and maintenance. Separate steps would show up distinctly in the GitHub Actions UI.
However, keeping the commands together is also valid, especially since they are closely related docker build operations. The current approach reduces workflow verbosity.
This comment represents a minor optimization that's more about personal preference than a clear improvement. It doesn't meet the threshold of being clearly required.
3. .github/workflows/goci.yaml:13
- Draft comment:
Avoid duplicating the complex 'if' condition across multiple jobs. Consider using YAML anchors or a reusable workflow to reduce maintenance overhead. - 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. .github/workflows/goci.yaml:50
- Draft comment:
Consider reviewing the sequential 'docker-build' commands. If build times are a concern, a matrix job or parallel steps might speed up the process. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
5. .github/workflows/goci.yaml:3
- Draft comment:
Confirm that restricting 'pull_request' triggers to the 'main' branch is intentional. If PRs on other branches should also trigger the workflow, update the branch filters accordingly. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
Workflow ID: wflow_9PcmlpcehVFFjOU3
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 3ae513c in 56 seconds
More details
- Looked at
41lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. .github/workflows/goci.yaml:37
- Draft comment:
Consider consolidating common steps (checkout, go-install, qemu-install, docker-build) between build-community and build-enterprise jobs to reduce duplication. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
2. .github/workflows/goci.yaml:55
- Draft comment:
Ensure that similar workflows (e.g., build-enterprise) use consistent configuration and consider potential refactoring into reusable workflows to adhere to DRY principles. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
3. .github/workflows/goci.yaml:37
- Draft comment:
Renaming 'build' to 'build-community' clarifies the job's intent. Ensure documentation (if needed) reflects this change. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
4. .github/workflows/goci.yaml:55
- Draft comment:
The 'build-enterprise' job duplicates many steps from 'build-community'. Consider extracting common steps into a reusable workflow to reduce duplication. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
Workflow ID: wflow_K56St7bBoKzcOVwT
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on b98d28c in 1 minute and 8 seconds
More details
- Looked at
132lines of code in4files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. pkg/query-service/Dockerfile:27
- Draft comment:
Duplicate COPY for frontend build detected. Remove one copy command to avoid redundancy. - 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. .github/workflows/goci.yaml:34
- Draft comment:
Renamed job 'build-community' to 'build' and merged enterprise build steps. Ensure that downstream CI configurations and documentation are updated accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to ensure that downstream CI configurations and documentation are updated, which violates the rule against asking the author to ensure things are updated or tested. It doesn't provide a specific code suggestion or highlight a specific issue with the code itself.
3. Makefile:127
- Draft comment:
Added '--build-arg ARCH="$"' to docker build commands. Verify that the pattern variable '$' expands as expected across all build targets. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
4. pkg/query-service/Dockerfile:27
- Draft comment:
Found duplicate COPY instruction for the frontend build. Remove the redundant line to avoid potential conflicts. - 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_NtlJF0l6WhmoycyX
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on b740211 in 1 minute and 31 seconds
More details
- Looked at
121lines of code in3files - Skipped
0files when reviewing. - Skipped posting
6drafted comments based on config settings.
1. Makefile:10
- Draft comment:
TMP_DIR and BIN_DIR removal: Ensure no tools or scripts depend on these removed directories. - 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. ee/query-service/Dockerfile:16
- Draft comment:
Ensure copying frontend build to /etc/signoz/web/ is expected and not conflicting with possible volume mounts. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
3. pkg/query-service/Dockerfile:16
- Draft comment:
Confirms copying frontend build directory to /etc/signoz/web/ is intentional for community version. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
4. Makefile:12
- Draft comment:
Removed TMP_DIR and BIN_DIR definitions and their mkdir rules. Ensure these were not referenced elsewhere and the cleanup won’t break any target that might expect them. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to ensure that the removal ofTMP_DIRandBIN_DIRdefinitions and theirmkdirrules won't break any targets that might expect them. This falls under asking the author to double-check things, which is against the rules.
5. ee/query-service/Dockerfile:21
- Draft comment:
The EXPOSE 8080 instruction was removed. If the service listens on port 8080, consider documenting it with an EXPOSE command for 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.
6. pkg/query-service/Dockerfile:21
- Draft comment:
Duplicate COPY of 'frontend/build/' was removed and chmod is now applied after copying. Verify that the frontend assets in '/etc/signoz/web/' are intact. Also consider whether an EXPOSE statement (e.g. EXPOSE 8080) is needed to document the container's network interface. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =50%<= threshold50%
The comment is asking the PR author to verify the integrity of frontend assets, which violates the rule against asking for confirmation or verification. However, the suggestion to consider adding an EXPOSE statement is a valid code suggestion, as it relates to documenting the container's network interface. I will approve the comment with a lower confidence due to the mixed content.
Workflow ID: wflow_xPoVa5uPVAlzvK8e
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 048f2a9 in 1 minute and 37 seconds
More details
- Looked at
16lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. .github/workflows/goci.yaml:51
- Draft comment:
Consider using a prebuilt image or caching to avoid installing packages at runtime. Installing gcc-aarch64-linux-gnu and musl-tools during the job may slow builds. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50%
The suggestion is reasonable from a CI optimization perspective - installing packages during each run does add time. However, the packages being installed are relatively small and specific to cross-compilation. Using a custom prebuilt image would add complexity and maintenance overhead. GitHub Actions already has some built-in caching for apt packages. Without knowing the full build context and frequency, it's hard to judge if this optimization would be worthwhile.
I may be underestimating the impact on build times. These packages could be quite large and the builds might run very frequently.
While true, the comment is more of an optimization suggestion rather than a clear issue that needs fixing. The current approach, while not optimal, is simple and functional.
The comment should be removed as it's more of an optional optimization suggestion without clear evidence that it would provide significant benefit in this specific case.
2. .github/workflows/goci.yaml:54
- Draft comment:
Optional: Combine 'apt-get update' and 'apt-get install' using '&&' to streamline the command, if build time optimization is needed. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
This is a CI workflow file where build time is important, but the optimization is minimal. The commands are already part of the same script block with set -ex for error handling. The current format is more readable and follows common practice. The suggestion is more of a style preference than a significant improvement.
The suggestion could save a few seconds in CI, and combining commands would prevent potential issues with apt cache becoming stale between commands.
The time savings are negligible, and the current format with separate lines is more maintainable and follows standard practice in CI workflows. The set -ex ensures proper error handling.
Delete the comment as it suggests a minor optimization that doesn't provide significant value and could reduce readability.
3. .github/workflows/goci.yaml:55
- Draft comment:
Consider pinning versions for gcc-aarch64-linux-gnu and musl-tools to ensure reproducible builds. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
While version pinning can help with reproducibility, this is in a CI environment that's already running on a specific Ubuntu version in GitHub Actions. The base image version provides some stability. Package versions from apt repositories are generally stable within a distribution version. This seems like being overly cautious without clear evidence of an actual problem.
Version pinning could prevent unexpected breaking changes if a package update introduces incompatibilities. CI environments should be as deterministic as possible.
GitHub Actions uses specific Ubuntu versions, providing a stable base. If there were known compatibility issues, they would be worth addressing, but this seems speculative without evidence of problems.
The comment should be removed as it suggests a speculative improvement without evidence of actual issues, and the existing setup already has reasonable stability through the GitHub Actions environment.
Workflow ID: wflow_osrej8EZx9Co6RsA
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 54519fe in 2 minutes and 7 seconds
More details
- Looked at
207lines of code in9files - Skipped
0files when reviewing. - Skipped posting
23drafted comments based on config settings.
1. .github/workflows/build.yaml:49
- Draft comment:
Consistently switching from DOCKER_TAG to VERSION. Ensure docs and dependent scripts reflect this change. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =50%<= threshold50%
This comment is asking the author to ensure that documentation and dependent scripts reflect a change fromDOCKER_TAGtoVERSION. It is indirectly asking for confirmation and verification, which violates the rule against asking the author to ensure things are tested or verified. However, it does point out a specific change that might need attention, which could be useful.
2. .github/workflows/gor-signoz-community.yaml:24
- Draft comment:
Changed frontend build command from 'build-frontend-static' to 'js-build'. Confirm that this update is intentional and update documentation if needed. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment asks the PR author to confirm their intention and update documentation, which violates the rules. It doesn't provide a specific code suggestion or ask for a specific test to be written. Therefore, it should be removed.
3. .github/workflows/gor-signoz.yaml:39
- Draft comment:
Frontend build command changed from 'build-frontend-static' to 'js-build'. Ensure dependent configuration and docs are updated accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to ensure that dependent configuration and documentation are updated, which violates the rule against asking the author to ensure things are tested or verified. It doesn't provide a specific suggestion or point out a specific issue with the code.
4. .github/workflows/staging-deployment.yaml:38
- Draft comment:
Replaced DOCKER_TAG with VERSION export. Verify that all downstream scripts and usage match the new variable name. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to verify that all downstream scripts and usage match the new variable name. This falls under the rule of not asking the PR author to double-check things or ensure behavior is intended. Therefore, this comment should be removed.
5. Makefile:31
- Draft comment:
Conditional assignment (using ?=) for DOCKER_REGISTRY variables improves flexibility. Confirm that this change does not break existing override behavior. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to confirm the behavior of a conditional assignment, which violates the rule against asking for confirmation of intention or behavior. It does not provide a specific suggestion or point out a clear issue.
6. deploy/docker/docker-compose.ha.yaml:180
- Draft comment:
Replaced image tag variable from DOCKER_TAG to VERSION; ensure deployment docs reflect this change. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the author to ensure that deployment documentation reflects a change in a variable name. This falls under asking the author to ensure something is updated, which is against the rules.
7. deploy/docker/docker-compose.testing.yaml:111
- Draft comment:
Updated image tag variable from DOCKER_TAG to VERSION; verify consistency across environments. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to verify consistency across environments, which violates the rule against asking the author to double-check things. It does not provide a specific suggestion or point out a specific issue.
8. deploy/docker/docker-compose.yaml:112
- Draft comment:
Replaced DOCKER_TAG with VERSION in image tag; double-check if this is aligned with release processes. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment asks the PR author to double-check if the change is aligned with release processes, which violates the rule against asking the author to confirm or double-check things. Therefore, this comment should be removed.
9. .github/workflows/build.yaml:51
- Draft comment:
Renamed DOCKER_TAG to VERSION in the enterprise job. Ensure downstream jobs and scripts expect the new variable. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to ensure that downstream jobs and scripts expect the new variable name. This falls under the rule of not asking the author to ensure behavior is intended or tested. Therefore, this comment should be removed.
10. .github/workflows/build.yaml:93
- Draft comment:
Renamed DOCKER_TAG to VERSION in the community job. Verify consistency with all image build steps. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to verify consistency, which violates the rule against asking for confirmation or verification. It does not provide a specific suggestion or point out a clear issue.
11. .github/workflows/gor-signoz-community.yaml:24
- Draft comment:
Updated frontend build command from 'build-frontend-static' to 'js-build'. Confirm that the Makefile target 'js-build' exists. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to confirm the existence of a Makefile target, which violates the rule against asking for confirmation or verification. It does not provide a specific code suggestion or improvement.
12. .github/workflows/gor-signoz.yaml:36
- Draft comment:
Replaced 'make build-frontend-static' with 'make js-build'. Ensure that frontend build targets are updated accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the author to ensure that frontend build targets are updated, which violates the rule against asking the author to ensure things. It doesn't provide a specific suggestion or point out a specific issue with the code.
13. .github/workflows/staging-deployment.yaml:38
- Draft comment:
Changed export from DOCKER_TAG to VERSION. Verify that deployment scripts consuming this variable are updated. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment asks the PR author to verify that deployment scripts are updated, which violates the rule against asking the author to ensure things are tested or verified. It doesn't provide a specific suggestion or point out a clear issue with the code.
14. .github/workflows/testing-deployment.yaml:38
- Draft comment:
Renamed DOCKER_TAG to VERSION in the testing deployment; ensure consistency across deployment configurations. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the author to ensure consistency across deployment configurations, which is similar to asking them to double-check or verify something. This violates the rule against asking the author to ensure or verify things.
15. Makefile:31
- Draft comment:
Changed assignment for DOCKER_REGISTRY_COMMUNITY and DOCKER_REGISTRY_ENTERPRISE to use '?=' for conditional assignment. Confirm no side effects if overridden. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to confirm that there are no side effects if the variables are overridden. This falls under the rule of not asking the author to confirm their intention or to ensure behavior is intended. Therefore, this comment should be removed.
16. deploy/docker/docker-compose.ha.yaml:178
- Draft comment:
Updated image tag reference to use VERSION instead of DOCKER_TAG. Ensure the deployment picks the correct image version. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to ensure that the deployment picks the correct image version, which violates the rule against asking the author to ensure behavior is intended. It doesn't provide a specific suggestion or point out a specific issue with the code.
17. deploy/docker/docker-compose.testing.yaml:113
- Draft comment:
Replaced DOCKER_TAG with VERSION in the image reference. Validate that testing deployments use the intended version. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to validate that testing deployments use the intended version, which falls under the rule of not asking the author to ensure the behavior is intended or to double-check things. Therefore, this comment should be removed.
18. deploy/docker/docker-compose.yaml:111
- Draft comment:
Changed image tag variable from DOCKER_TAG to VERSION. Confirm that this change propagates to all deployment environments. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment asks the PR author to confirm that a change propagates to all deployment environments, which violates the rule against asking the author to confirm or ensure things. It doesn't provide a specific code suggestion or ask for a test to be written. Therefore, it should be removed.
19. .github/workflows/gor-signoz.yaml:39
- Draft comment:
Trivial: There's an extra space in the command at line 39 ('run: make js-build'). Consider removing the additional space to ensure consistency with the rest of the file. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While the comment is technically correct about the inconsistency, it's an extremely minor formatting issue that doesn't affect functionality. The rules state not to make comments that are obvious or unimportant. This feels like a trivial nitpick that doesn't improve code quality in any meaningful way.
The extra space could be seen as a violation of consistent formatting, which some teams take seriously. YAML can be sensitive to spacing in some contexts.
While YAML is space-sensitive, this particular space afterrun:is not in a position that would affect YAML parsing. It's purely cosmetic.
Delete this comment as it points out an extremely minor formatting issue that doesn't impact functionality or code quality in any meaningful way.
20. .github/workflows/testing-deployment.yaml:49
- Draft comment:
Typographical Error: On line 49, the word 'scenerio' appears in the comment. Please correct it to 'scenario' for 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.
21. deploy/docker/docker-compose.ha.yaml:12
- Draft comment:
Typo in comment: 'addding non LTS version' should be corrected to 'adding non LTS version'. - 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.
22. deploy/docker/docker-compose.testing.yaml:12
- Draft comment:
Typographical error: 'addding' on line 12 should be corrected to 'adding'. - 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.
23. deploy/docker/docker-compose.yaml:12
- Draft comment:
Typo found: 'addding' should be corrected to 'adding'. - 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_ejyxgY3delbIukpr
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on b86853b in 1 minute and 22 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. Makefile:5
- Draft comment:
Ensure that using an absolute path via $(shell pwd) is intentional as it changes the behavior from relative '.'; verify all dependent commands work with absolute paths. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the author to ensure that the use of an absolute path is intentional and to verify dependent commands. This violates the rule against asking the author to confirm their intention or verify behavior. Therefore, this comment should be removed.
Workflow ID: wflow_H074LcO38CxRdOW9
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 7d89156 in 1 minute and 3 seconds
More details
- Looked at
42lines of code in2files - Skipped
0files when reviewing. - Skipped posting
6drafted comments based on config settings.
1. ee/query-service/.goreleaser.yaml:33
- Draft comment:
Ensure the version prefix 'v' in ldflags matches the expected version format across components. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to ensure that a version prefix matches the expected format. It is not a specific code suggestion or a request for a test, and it doesn't provide a clear actionable item. It seems to be more of a reminder or a request for confirmation, which violates the rules.
2. pkg/query-service/.goreleaser.yaml:33
- Draft comment:
Confirm that the 'v' prefix in version ldflags and release naming aligns with the overall versioning scheme. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking for confirmation about the alignment of the 'v' prefix in version ldflags and release naming with the overall versioning scheme. It is asking the PR author to confirm their intention, which violates the rule against asking for confirmation or intention.
3. ee/query-service/.goreleaser.yaml:33
- Draft comment:
Ensure .Version doesn't already include 'v' to avoid double-prefixing in ldflags. - Reason this comment was not posted:
Confidence changes required:20%<= threshold50%
None
4. ee/query-service/.goreleaser.yaml:66
- Draft comment:
Using 'v' prefix in release name_template is consistent; verify downstream consumers expect this format. - Reason this comment was not posted:
Confidence changes required:20%<= threshold50%
None
5. pkg/query-service/.goreleaser.yaml:33
- Draft comment:
Confirm that prepending 'v' in ldflags doesn't duplicate an existing 'v' in .Version. - Reason this comment was not posted:
Confidence changes required:20%<= threshold50%
None
6. pkg/query-service/.goreleaser.yaml:64
- Draft comment:
Ensure release name_template with the 'v' prefix aligns with overall versioning strategy. - Reason this comment was not posted:
Confidence changes required:20%<= threshold50%
None
Workflow ID: wflow_GIHfOCst0psLqtj1
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on b2a7e71 in 1 minute and 51 seconds
More details
- Looked at
102lines of code in4files - Skipped
0files when reviewing. - Skipped posting
20drafted comments based on config settings.
1. .github/workflows/build.yaml:17
- Draft comment:
Updated setup-go to v5; verify compatibility with v5 changes. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to verify compatibility with a new version of a dependency, which is against the rules. It doesn't provide a specific suggestion or point out a specific issue with the code.
2. .github/workflows/build.yaml:72
- Draft comment:
Updated setup-go to v5 in the community job; ensure consistency across workflows. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative and does not provide any actionable feedback or suggestions. It simply states what was done without offering any guidance or asking for clarification.
3. Makefile:119
- Draft comment:
Switched build argument from ARCH to TARGETARCH; confirm Dockerfile expects TARGETARCH. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to confirm that the Dockerfile expectsTARGETARCHinstead ofARCH. This falls under the rule of not asking the PR author to confirm their intention or to double-check things. Therefore, this comment should be removed.
4. Makefile:127
- Draft comment:
Removal of explicit --build-arg for docker buildx commands is intentional; buildx auto-populates TARGETARCH. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative, explaining the reason behind a change. It doesn't provide a suggestion or ask for confirmation about a specific code change.
5. Makefile:138
- Draft comment:
Updated docker build command for enterprise to use TARGETARCH instead of ARCH. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative, as it only describes a change made in the code without providing any actionable feedback or suggestions. It does not ask for confirmation or suggest improvements.
6. ee/query-service/Dockerfile:6
- Draft comment:
Renamed ARG from ARCH to TARGETARCH; ensure builds pass the correct argument. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =50%<= threshold50%
This comment is asking the author to ensure that builds pass the correct argument after a rename. It indirectly asks for confirmation of intention, which violates the rules. However, it does provide a specific suggestion about the change made, which could be useful.
7. ee/query-service/Dockerfile:13
- Draft comment:
Updated COPY command to use TARGETARCH for path resolution. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative and does not provide any actionable feedback or suggestions. It simply states what was changed without offering any insight or guidance.
8. pkg/query-service/Dockerfile:6
- Draft comment:
Renamed ARG from ARCH to TARGETARCH; verify alignment with build process. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the author to verify alignment with the build process, which is against the rules of asking the author to double-check things. It doesn't provide a specific suggestion or point out a clear issue.
9. pkg/query-service/Dockerfile:13
- Draft comment:
Updated COPY command to use TARGETARCH for consistent artifact retrieval. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative, explaining what was changed without providing any actionable feedback or suggestions. It does not align with the rules for useful comments.
10. .github/workflows/build.yaml:17
- Draft comment:
Updated actions/setup-go to v5; verify that v5 supports go-version 1.22 without breaking changes. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
11. .github/workflows/build.yaml:72
- Draft comment:
Updated actions/setup-go to v5 here as well; ensure compatibility with the specified go version. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
12. Makefile:119
- Draft comment:
Changed docker build argument from ARCH to TARGETARCH for the community image to match the Dockerfile. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
13. Makefile:138
- Draft comment:
Updated docker build argument to use TARGETARCH in the enterprise image build for consistency. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
14. Makefile:127
- Draft comment:
Removed explicit '--build-arg ARCH'; confirm that Buildx auto-injects TARGETARCH as expected during multi-arch builds. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
15. Makefile:145
- Draft comment:
Ensure Buildx for the enterprise build auto-injects TARGETARCH since the explicit build argument was removed. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
16. ee/query-service/Dockerfile:6
- Draft comment:
Renamed ARG from ARCH to TARGETARCH to align with the build scripts and Makefile. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
17. ee/query-service/Dockerfile:13
- Draft comment:
Updated COPY command to use TARGETARCH, ensuring the correct binary path is referenced. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
18. pkg/query-service/Dockerfile:6
- Draft comment:
Renamed ARG from ARCH to TARGETARCH for consistency with the updated build process. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
19. pkg/query-service/Dockerfile:13
- Draft comment:
Updated COPY command to reference TARGETARCH, matching the new naming convention. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
20. Makefile:84
- Draft comment:
The build commands (lines 84, 86, 97, and 99) use the build tag 'timetzdata'. Please confirm if this is intentional or if it should be corrected (for example, to 'tzdata') to avoid potential confusion. - 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_nG3AgIMOkUqIiBr5
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on eb76779 in 1 minute and 47 seconds
More details
- Looked at
64lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. .github/workflows/build.yaml:44
-
Draft comment:
Consider quoting variable expansions in the bash script to avoid word splitting issues. For example, use "${GH_REF}" instead of just ${GH_REF}. -
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
- The value being assigned comes from GitHub Actions context ${{ github.ref }} which is already safely handled by the Actions runner. 2. The variable is only used in string operations with ${GH_REF#$PREFIX} which is safe. 3. There's no word splitting risk here since github.ref is a single string value. 4. The code works as-is and adding quotes would be overly defensive.
The comment raises a generally good bash practice, but am I being too dismissive of defensive programming in CI scripts?
While defensive programming is good, in this specific context with GitHub Actions expressions, the extra quotes add no real safety benefit and just add noise.
The comment should be removed as it suggests a change that adds no practical value in this specific GitHub Actions context.
2. .github/workflows/build.yaml:44
-
Draft comment:
In the 'github-ref-info' step (enterprise job), consider adding 'set -e' at the beginning of the run block to ensure the script exits on error. Also, this inline logic duplicates version extraction that appears later in the community job—extracting it to a reusable action or separate script could reduce duplication. -
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
Theset -esuggestion is valid but not critical since this is a fairly simple script that's unlikely to fail silently. The duplication observation is accurate but may be intentionally kept separate for job isolation. Moving to a reusable action adds complexity and coupling between jobs. Neither issue is severe enough to require immediate action.
The comment points out real issues, but are they important enough to fix? The current approach, while not perfect, is simple and functional.
While technically correct, these suggestions represent optional improvements rather than critical issues that need fixing.
The comment should be removed as it suggests nice-to-have improvements rather than necessary changes.
3. .github/workflows/build.yaml:85
-
Draft comment:
The 'github-ref-info' step for the community job duplicates the version extraction logic. Consider refactoring to avoid code repetition by reusing a common script or action. -
Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_cB6pv9oqDLgdpxPg
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
e63d1b8 to
12763c1
Compare
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on cd0b9ab in 52 seconds
More details
- Looked at
101lines of code in6files - Skipped
0files when reviewing. - Skipped posting
14drafted comments based on config settings.
1. ee/query-service/Dockerfile:14
- Draft comment:
Updated config and template paths; verify build context/root directory consistency. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to verify the build context and root directory consistency, which falls under the rule of not asking the author to double-check or ensure behavior. It doesn't provide a specific suggestion or point out a clear issue.
2. pkg/query-service/Dockerfile:14
- Draft comment:
Updated config/template paths; ensure new directory structure is correctly set in the build context. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to ensure that the new directory structure is correctly set in the build context. This falls under the rule of not asking the author to ensure or verify things, which is not allowed.
3. pkg/query-service/config/alerts.yml:1
- Draft comment:
Removal of alerts configuration. Confirm that alert rules are intentionally deprecated or moved. - 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/config/cache-config.yml:1
- Draft comment:
Cache config removal; ensure that default caching behavior is managed elsewhere if needed. - 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/config/prometheus.yml:1
- Draft comment:
Prometheus config file content removed; double-check that the new config in ./config/prometheus.yml covers all required settings. - 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/templates/invitation_email_template.html:1
- Draft comment:
Invitation email template removed; ensure that email invitation workflows are updated to use the new template path or mechanism. - 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. ee/query-service/Dockerfile:14
- Draft comment:
Ensure the new relative path './config/prometheus.yml' is correct and available in all build contexts, as it replaces the previous pkg/query-service path. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to ensure that a path is correct and available in all build contexts. This falls under the rule of not asking the author to double-check things or ensure behavior, which is not allowed.
8. ee/query-service/Dockerfile:15
- Draft comment:
Verify that copying only the './templates/email' directory (instead of the full templates folder) meets all template requirements. Document if other templates are needed. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to verify and document something, which is against the rules. It doesn't provide a specific suggestion or point out a clear issue with the code.
9. pkg/query-service/Dockerfile:14
- Draft comment:
Confirm that the updated path './config/prometheus.yml' is correctly located for the community build as it replaces the previous pkg/query-service config path. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to confirm the correctness of a path change, which is against the rules. It doesn't provide a specific suggestion or highlight a clear issue with the code.
10. pkg/query-service/Dockerfile:15
- Draft comment:
Ensure that copying './templates/email' remains sufficient and that no additional template files (from the former templates directory) are now needed for full functionality. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to ensure that a specific behavior is intended, which violates the rule against asking for confirmation of intention or ensuring behavior. It does not provide a specific code suggestion or ask for a specific test to be written.
11. pkg/query-service/config/alerts.yml:1
- Draft comment:
The alerts configuration file has been removed. Confirm that alert rules are now managed elsewhere and that this change won't break the alerting functionality. - 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.
12. pkg/query-service/config/cache-config.yml:1
- Draft comment:
Removed cache configuration file. Verify that caching settings are now handled by an alternative mechanism to avoid unintended side effects. - 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.
13. pkg/query-service/config/prometheus.yml:1
- Draft comment:
Prometheus global configuration has been removed. Ensure that necessary scrape, alerting, and remote_read settings are provided elsewhere according to the new package 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.
14. pkg/query-service/templates/invitation_email_template.html:1
- Draft comment:
The invitation email template has been removed. Confirm that its functionality is either deprecated or replaced by the new email templates in './templates/email'. - 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_6bdWssp8jZShYFLX
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on fa3b58e in 1 minute and 22 seconds
More details
- Looked at
55lines of code in2files - Skipped
0files when reviewing. - Skipped posting
6drafted comments based on config settings.
1. Makefile:61
- Draft comment:
Ensure the new enterprise target (go-run-enterprise) and its added flags (e.g., --use-logs-new-schema, --use-trace-new-schema) are documented and intended for all situations. Also, confirm that updating the config path from './pkg/query-service/config/prometheus.yml' to './conf/prometheus.yml' is consistent with other parts of the codebase. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to ensure that the new enterprise target and its flags are documented and intended for all situations, which violates the rule against asking for confirmation of intention or ensuring behavior. Additionally, it asks to confirm consistency with other parts of the codebase, which is also against the rules.
2. docs/contributing/development.md:64
- Draft comment:
The backend run command was updated from 'make run-go' to 'make go-run-community'. Please verify that the documentation clearly distinguishes between the community and enterprise backends. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to verify documentation, which is against the rules. It doesn't provide a specific code suggestion or point out a potential issue with the code itself.
3. Makefile:61
- Draft comment:
Renamed target 'go-run' to 'go-run-enterprise' for clarity. Ensure corresponding documentation reflects this change if needed. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
4. Makefile:77
- Draft comment:
The enterprise target omits several environment variables (e.g., SIGNOZ_TELEMETRYSTORE_PROVIDER and DSN, SIGNOZ_JWT_SECRET, SIGNOZ_ALERTMANAGER_PROVIDER) that are set in the community target. Confirm if this difference is intentional. - Reason this comment was not posted:
Comment was on unchanged code.
5. Makefile:72
- Draft comment:
The config file path changed from './pkg/query-service/config/prometheus.yml' to './conf/prometheus.yml'. Verify that this new path is correct and consistent with your project structure. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
6. docs/contributing/development.md:64
- Draft comment:
Updated the command from 'make run-go' to 'make go-run-community'. Ensure that users are aware when to use the community target versus the enterprise target. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
Workflow ID: wflow_c1wOEO9CfYomagv8
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 6a24633 in 26 seconds
More details
- Looked at
26lines of code in2files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. ee/query-service/Dockerfile:14
- Draft comment:
Updated path for prometheus configuration looks correct if the file was moved from ./config to ./conf. Ensure this change is intentional. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
2. pkg/query-service/Dockerfile:14
- Draft comment:
Verified change: using './conf/prometheus.yml' is consistent with new structure. Confirm that all environments have this new structure. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
3. ee/query-service/Dockerfile:14
- Draft comment:
Ensure the updated path for prometheus.yml (changed from config/ to conf/) is intentional and aligned with documentation. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
4. pkg/query-service/Dockerfile:14
- Draft comment:
Verify the new path for prometheus.yml (now in conf/) matches project conventions and documentation. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
Workflow ID: wflow_up6rq1AM5mcg9sCS
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on b7d1598 in 45 seconds
More details
- Looked at
20lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. .github/workflows/commitci.yaml:12
- Draft comment:
The lint job was removed. Ensure that commit message linting is handled elsewhere if needed. This change looks intentional, but double-check that no critical lint checks are missed. - 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. .github/workflows/commitci.yaml:29
- Draft comment:
The removal of the 'lint' job (which used wagoid/commitlint-github-action@v5) might impact commit message linting. If commit linting is still required, ensure that it is integrated elsewhere. Otherwise, confirm that this removal is intentional. - 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_zb6GtVTjRlzSkJQS
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
Summary
add the new version package
Related Issues / PR's
Part of #6805
Screenshots
NA
Affected Areas and Manually Tested Areas
Important
Introduce a new versioning package and refactor build and deployment processes, including Docker image builds and version banner feature.
.github/workflows/build.yamlto streamline Docker image builds forenterpriseandcommunity..github/workflows/push.yamland integrate its functionality intobuild.yaml.Makefileto support new build and Docker image targets, includinggo-build-community,go-build-enterprise,docker-build-community, anddocker-build-enterprise.pkg/versionpackage for managing build version information.conf/example.yamlandpkg/version/config.go.pkg/query-service/main.goandpkg/signoz/signoz.goto use new versioning package.pkg/query-service/config/alerts.yml,pkg/query-service/config/cache-config.yml,pkg/query-service/config/prometheus.yml, andpkg/query-service/templates/invitation_email_template.html.Dockerfileinee/query-serviceandpkg/query-serviceto align with new build process.make build-frontend-statictomake js-buildingor-signoz-community.yamlandgor-signoz.yaml.This description was created by
for b7d1598. It will automatically update as commits are pushed.