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

feat: cns logger v2 [1/2] #3437

Merged
merged 5 commits into from
Feb 27, 2025
Merged

feat: cns logger v2 [1/2] #3437

merged 5 commits into from
Feb 27, 2025

Conversation

rbtr
Copy link
Contributor

@rbtr rbtr commented Feb 20, 2025

Adds v2 logger package for CNS. [1/2]

The v2 logger is built with zap and offers structured logging, derived contexts, multiple output targets and formats including stdout, files, ETW, and AppInsights.
The logger constructor is opinionated and will initialize targets and formats fully based on what is supported per-platform. The logger is intended to be used directly.

This pt. 1 change is strictly the addition of the v2 logger package machinery.

@Copilot Copilot bot review requested due to automatic review settings February 20, 2025 23:03
@rbtr rbtr requested review from a team as code owners February 20, 2025 23:03
@rbtr
Copy link
Contributor Author

rbtr commented Feb 20, 2025

The ETW changes will drop out as #3433 merges.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 6 out of 12 changed files in this pull request and generated 4 comments.

Files not reviewed (6)
  • go.mod: Language not supported
  • cns/logger/cnslogger_windows.go: Evaluated as low risk
  • cni/log/logger_windows.go: Evaluated as low risk
  • zapetw/core_windows.go: Evaluated as low risk
  • cns/logger/v2/logger_linux.go: Evaluated as low risk
  • cns/logger/v2/logger_windows.go: Evaluated as low risk
Comments suppressed due to low confidence (1)

cns/logger/v2/cores/etw_windows.go:17

  • The comment should specify that the second return value is a function to close the provider for clarity.
// The first return is the core, the second is a function to close the sink.

@rbtr rbtr force-pushed the feat/cns-zapai-migration branch from 5e2ebf4 to dec6d16 Compare February 20, 2025 23:14
@rbtr rbtr self-assigned this Feb 21, 2025
@rbtr rbtr added the cns Related to CNS. label Feb 21, 2025
@rbtr rbtr force-pushed the feat/cns-zapai-migration branch 2 times, most recently from 7b3c7cc to 0294ccd Compare February 21, 2025 00:19
rbtr added 2 commits February 21, 2025 21:02
Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>
Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>
@rbtr rbtr force-pushed the feat/cns-zapai-migration branch from 0294ccd to d3bbfc8 Compare February 21, 2025 21:02
rbtr added 3 commits February 21, 2025 23:53

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>
lints
Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>
@rbtr
Copy link
Contributor Author

rbtr commented Feb 25, 2025

/azp run Azure Container Networking PR

@rbtr rbtr requested a review from jackieluc February 25, 2025 20:06
@jpayne3506
Copy link
Contributor

/azp run Azure Container Networking PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rbtr rbtr enabled auto-merge February 27, 2025 00:18
@rbtr rbtr added this pull request to the merge queue Feb 27, 2025
Merged via the queue into master with commit b9494d9 Feb 27, 2025
14 checks passed
@rbtr rbtr deleted the feat/cns-zapai-migration branch February 27, 2025 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cns Related to CNS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants