Skip to content

Comments

chore: remove otlp zap encoder#7408

Merged
therealpandey merged 1 commit intomainfrom
part-of-6805-2
Mar 23, 2025
Merged

chore: remove otlp zap encoder#7408
therealpandey merged 1 commit intomainfrom
part-of-6805-2

Conversation

@therealpandey
Copy link
Member

@therealpandey therealpandey commented Mar 22, 2025

Summary

remove otlp zap encoder.

Related Issues / PR's

Already archived https://github.com/SigNoz/zap_otlp
Part of #6805

Screenshots

NA

Affected Areas and Manually Tested Areas


Important

Remove OTLP zap encoder and related dependencies from the codebase.

  • Behavior:
    • Removed OTLP zap encoder from initZapLog() in main.go. No longer supports OTLP log export.
    • Removed enableQueryServiceLogOTLPExport flag from main() in main.go.
  • Dependencies:
    • Removed github.com/SigNoz/zap_otlp/zap_otlp_encoder and github.com/SigNoz/zap_otlp/zap_otlp_sync from go.mod.
    • Removed google.golang.org/grpc from go.mod and added it as an indirect dependency.
  • Misc:
    • Updated initZapLog() to use CapitalColorLevelEncoder for log level encoding.

This description was created by Ellipsis for ab97247. 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.

❌ Changes requested. Reviewed everything up to ab97247 in 1 minute and 44 seconds

More details
  • Looked at 156 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 7 drafted comments based on config settings.
1. ee/query-service/main.go:25
  • Draft comment:
    Ensure all OTLP zap encoder logic/config has been completely removed, and that the logging configuration remains correct.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to ensure that a specific part of the code has been completely removed and that the logging configuration remains correct. This falls under the rule of not asking the PR author to ensure or verify things, which is not allowed.
2. go.mod:11
  • Draft comment:
    Removed OTLP zap encoder dependencies; verify that no code still references these packages.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to verify that no code still references the removed dependencies. This falls under the rule of not asking the author to double-check things, which is not allowed.
3. go.sum:1
  • Draft comment:
    The go.sum changes show removal of OTLP zap encoder hashes; ensure all related entries are cleaned up.
  • 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. go.mod:8
  • Draft comment:
    Removed dependencies on the OTLP zap encoder modules (e.g. github.com/SigNoz/zap_otlp/zap_otlp_encoder and zap_otlp_sync). Verify that logger configuration now uses the standard encoder and that no residual references remain.
  • 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. go.sum:1
  • Draft comment:
    Ensure that the removal of OTLP zap encoder dependencies in go.sum is complete and that no outdated checksums remain.
  • 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. ee/query-service/main.go:59
  • Draft comment:
    The flag 'use-logs-new-schema' at line 59 has a description that refers to 'logs_v2 schema for logs'. Please clarify whether this flag is for enabling a new schema or the legacy v2 schema, and update the description accordingly.
  • 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/main.go:56
  • Draft comment:
    Consider renaming the variable 'gatewayUrl' (declared at line 56) to 'gatewayURL' to follow standard acronym capitalization conventions.
  • 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_mU3MbvKzQFu6yGfQ


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.

@therealpandey therealpandey merged commit 089f128 into main Mar 23, 2025
15 of 17 checks passed
@therealpandey therealpandey deleted the part-of-6805-2 branch March 23, 2025 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants