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

MG-2059 - Set correct log level depending on error #2196

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

nyagamunene
Copy link
Contributor

What type of PR is this?

This is a refactor because it sets correct log level depending on error.

What does this do?

It fixes the correct log level for the logs. Currently all log are warnings.

Which issue(s) does this PR fix/relate to?

Have you included tests for your changes?

No.

Did you document any new/modified feature?

No.

Notes

@nyagamunene nyagamunene force-pushed the MG2059-ErrorRefactoring branch 2 times, most recently from 533c151 to a596fd9 Compare April 29, 2024 07:23
@nyagamunene nyagamunene marked this pull request as ready for review April 29, 2024 07:23
Copy link
Contributor

@arvindh123 arvindh123 left a comment

Choose a reason for hiding this comment

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

Please check the following files, I found some error are logged as Warn :

auth/api/logging.go L30 to L128 https://github.com/nyagamunene/magistrala/blob/a596fd9abbf62be732b9185855def1f492f3baeb/auth/api/logging.go#L37-L129

bootstrap/api/logging.go

bootstrap/postgres/configs.go

certs/api/logging.go

cmd/opcua/main.go

coap/api/transport.go

consumers/notifiers/api/logging.go

consumers/writers/api/logging.go

internal/groups/api/logging.go

consumers/writers/api/logging.go

internal/groups/api/logging.go

lora/api/logging.go

invitations/middleware/logging.go

opcua/api/logging.go

pkg/events/redis/subscriber.go

things/api/logging.go

twins/api/logging.go

users/api/logging.go

ws/api/endpoints.go

Copy link
Member

@rodneyosodo rodneyosodo left a comment

Choose a reason for hiding this comment

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

Some errors should be logged with Warn while others should be logged with Error. This is a simple explanation of how to do it https://stackoverflow.com/a/2031195 . Also, consider using ErrorContext and pass in the context rather than Error

@nyagamunene nyagamunene force-pushed the MG2059-ErrorRefactoring branch 2 times, most recently from 95f66c4 to 7022dd0 Compare May 9, 2024 08:41
@@ -34,7 +34,7 @@ func (lm *loggingMiddleware) ListObjects(ctx context.Context, pr auth.PolicyReq,
}
if err != nil {
args = append(args, slog.Any("error", err))
lm.logger.Warn("List objects failed to complete successfully", args...)
lm.logger.ErrorContext(ctx, "List objects failed to complete successfully", args...)
Copy link
Member

Choose a reason for hiding this comment

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

Revert all errors to WarnContext

@@ -27,7 +27,7 @@ func handshake(ctx context.Context, svc ws.Service) http.HandlerFunc {
}
conn, err := upgrader.Upgrade(w, r, nil)
if err != nil {
logger.Warn(fmt.Sprintf("Failed to upgrade connection to websocket: %s", err.Error()))
logger.Error(fmt.Sprintf("Failed to upgrade connection to websocket: %s", err.Error()))
Copy link
Member

Choose a reason for hiding this comment

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

Use parent context

@@ -62,7 +62,7 @@ func decodeRequest(r *http.Request) (connReq, error) {

channelParts := channelPartRegExp.FindStringSubmatch(r.RequestURI)
if len(channelParts) < 2 {
logger.Warn("Empty channel id or malformed url")
logger.Error("Empty channel id or malformed url")
Copy link
Member

Choose a reason for hiding this comment

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

Use request context

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene nyagamunene changed the title MG-2059-Set correct log level depending on error MG-2059 - Set correct log level depending on error May 21, 2024
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
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.

Set correct log level depending on error
3 participants