Skip to content

Conversation

@alexandernorth
Copy link
Contributor

In order to help with observability, this PR migrates the existing logging framework to a logr.Logger based one, which is passed to various functions through the context.
This has the advantage that fields can be added to the logger as the application proceeds, adding relevant context to the log output and grouping together messages which belong to the same request/goroutine/...
It is also using structured logging which can help when integrating logs into observability tools.

This also refactors the netconf set function to have only one version of the code to handle setting to running or candidate stores, rather than two separate functions

@alexandernorth alexandernorth force-pushed the feature/logging-refactoring branch 3 times, most recently from 451677d to cfbba53 Compare June 23, 2025 10:40
@alexandernorth alexandernorth force-pushed the feature/logging-refactoring branch 3 times, most recently from c875f90 to c12e5ac Compare September 29, 2025 12:00
@alexandernorth alexandernorth marked this pull request as ready for review October 1, 2025 17:27
Copy link
Contributor

@henderiw henderiw left a comment

Choose a reason for hiding this comment

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

lgtm

@alexandernorth alexandernorth force-pushed the feature/logging-refactoring branch from 40b6968 to 1aa4c27 Compare October 9, 2025 13:06
@steiler
Copy link
Collaborator

steiler commented Oct 23, 2025

LGTM

@alexandernorth alexandernorth force-pushed the feature/logging-refactoring branch from 1146035 to a9afff7 Compare October 28, 2025 07:55
…r.Logger and passed through context

refactor setToDevice

add context to convert-like functions

aligned logs for incoming gnmi requests, adding method name to logger name and using json for format

added additional names for sbi/target funcs

added ctx params

migrate to slog

implement feedback

use MarshalOptions to stop multiline proto formatting

remove WithName grpc
@alexandernorth alexandernorth force-pushed the feature/logging-refactoring branch from a9afff7 to 8f05f11 Compare October 28, 2025 08:02
@steiler steiler self-requested a review October 28, 2025 08:39
Copy link
Collaborator

@steiler steiler left a comment

Choose a reason for hiding this comment

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

lgtm

@steiler steiler merged commit 928dc96 into sdcio:main Oct 28, 2025
6 checks passed
@codecov
Copy link

codecov bot commented Oct 28, 2025

Codecov Report

❌ Patch coverage is 7.35010% with 479 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/server/schema.go 0.00% 105 Missing ⚠️
pkg/server/datastore.go 0.00% 41 Missing ⚠️
pkg/datastore/target/nc.go 9.09% 40 Missing ⚠️
pkg/server/server.go 0.00% 40 Missing ⚠️
pkg/datastore/transaction_rpc.go 0.00% 37 Missing ⚠️
pkg/datastore/datastore_rpc.go 0.00% 32 Missing ⚠️
pkg/server/transaction.go 0.00% 32 Missing ⚠️
pkg/utils/leaf_convert.go 0.00% 20 Missing ⚠️
pkg/datastore/target/gnmi.go 0.00% 18 Missing ⚠️
pkg/datastore/deviations.go 0.00% 16 Missing ⚠️
... and 22 more

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@alexandernorth alexandernorth deleted the feature/logging-refactoring branch October 28, 2025 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants