owncloud / ocis

:atom_symbol: ownCloud Infinite Scale Stack
https://doc.owncloud.com/ocis/next/
Apache License 2.0
1.42k stars 183 forks source link

Possible log flood when log is set to "debug" level #8274

Open jvillafanez opened 10 months ago

jvillafanez commented 10 months ago

Describe the bug

Periodic debug logs appears without doing anything.

{"level":"debug","service":"storage-system","service":{"name":"com.owncloud.api.storage-system","version":"5.1.0-prealpha+0ba030949","metadata":null,"endpoints":[],"nodes":[{"id":"com.owncloud.api.storage-system-3cdb2150-13a6-4146-bd83-035c7065d4a5","address":"127.0.0.1:9215","metadata":{"protocol":"grpc","registry":"cache","server":"grpc","transport":"grpc"}}]},"time":"2024-01-23T16:59:12Z","line":"github.com/owncloud/ocis/v2/ocis-pkg/registry/register.go:30","message":"refreshing external service-registration"}
{"level":"debug","service":"app-registry","service":{"name":"com.owncloud.api.app-registry","version":"5.1.0-prealpha+0ba030949","metadata":null,"endpoints":[],"nodes":[{"id":"com.owncloud.api.app-registry-196dec3f-9a10-4b8d-9c1d-b0e54f44753b","address":"127.0.0.1:9242","metadata":{"protocol":"grpc","registry":"cache","server":"grpc","transport":"grpc"}}]},"time":"2024-01-23T16:59:12Z","line":"github.com/owncloud/ocis/v2/ocis-pkg/registry/register.go:30","message":"refreshing external service-registration"}

Steps to reproduce

  1. Just set log level to "debug"

Expected behavior

Log should appear only when the service is actually registered. If the service is already there, re-registering is meaningless and shouldn't happen, and the log shouldn't appear

Actual behavior

Periodic actions shouldn't log meaningless things to avoid flooding the logs, specially if there is no user interaction.

Setup

Please describe how you started the server and provide a list of relevant environment variables or configuration files.

```console OCIS_LOG_LEVEL=debug ```

Additional context

Log comes from https://github.com/owncloud/ocis/blob/c9a77eb43f1f2d4591c9b8920396437b758ad624/ocis-pkg/registry/register.go#L30

Taking into account that the log happens on a timer in a different thread, it makes sense to check whether the service is already in the registry. If the service isn't registered, or has been unregistered, write a debug log when the service is successfully registered.

We could also remove that log. We already have a log when the service is registered the first time, which shouldn't give any problems. There are logs when the registration fails, including the timed re-registration, so we can assume that there is no problem if no log happens. Therefore, the "refreshing registration" log is redundant and can be removed.

kobergj commented 10 months ago

Log should appear only when the service is actually registered.

We use a ttl on the registry, so services need to register periodically. So this log means the service has actually (re-) registered. However I'd be fine with only logging in error case

jvillafanez commented 10 months ago

Removing the log seems to be the easiest option.

Alternative 1 could be to check whether the service it already registered, but since we have to re-register the service anyway to update the ttl, we'd be checking just to show the log, so it's likely a big waste.

Alternative 2 could be to implement a specific method to update the ttl and / or register, something like UpdateTTLOrRegister(...) so the method could return additional information, not just the error. This doesn't seem a short term plan though.