From 2ba693f040967a0ba6dc1030c560b182b9720cf3 Mon Sep 17 00:00:00 2001 From: Vibhu Pandey Date: Sun, 25 May 2025 11:40:39 +0530 Subject: [PATCH] chore(linter): add more linters and deprecate zap (#8034) * chore(linter): add more linters and deprecate zap * chore(linter): add more linters and deprecate zap * chore(linter): add more linters and deprecate zap * chore(linter): add more linters and deprecate zap --- .golangci.yml | 29 ++++++++++++++ ee/http/middleware/api_key.go | 9 +++-- ee/licensing/httplicensing/provider.go | 11 ++---- ee/modules/user/impluser/module.go | 6 --- ee/query-service/app/api/api.go | 2 +- ee/query-service/app/server.go | 20 +++++----- .../legacyalertmanager/provider.go | 2 +- pkg/alertmanager/service.go | 12 +++--- pkg/apis/fields/api.go | 7 ++-- pkg/cache/memorycache/provider.go | 2 +- pkg/cache/rediscache/provider.go | 20 ++++------ pkg/cache/rediscache/provider_test.go | 20 +++++----- pkg/emailing/smtpemailing/provider.go | 2 +- .../templatestore/filetemplatestore/store.go | 6 +-- pkg/http/middleware/analytics.go | 33 ++++------------ pkg/http/middleware/auth.go | 10 +---- pkg/http/middleware/cache_test.go | 3 ++ pkg/http/middleware/logging.go | 38 +++++++++---------- pkg/http/middleware/timeout.go | 15 +++----- pkg/http/middleware/timeout_test.go | 10 +++-- pkg/http/render/render_test.go | 6 +++ pkg/http/server/server.go | 16 ++++---- pkg/query-service/app/server.go | 18 ++++----- .../integration/filter_suggestions_test.go | 2 +- .../signoz_cloud_integrations_test.go | 3 +- .../integration/signoz_integrations_test.go | 3 +- .../rulestore/sqlrulestore/maintenance.go | 4 -- pkg/ruler/rulestore/sqlrulestore/rule.go | 4 -- pkg/smtp/client/smtp.go | 2 +- pkg/sqlmigration/026_update_integrations.go | 2 - pkg/sqlstore/sqlstorehook/logging.go | 6 +-- pkg/telemetrylogs/filter_expr_logs_test.go | 2 +- pkg/telemetrymetadata/metadata.go | 9 +++-- pkg/telemetrymetadata/metadata_test.go | 3 ++ pkg/types/domain.go | 2 - pkg/types/integration.go | 10 ++--- pkg/types/ruletypes/maintenance.go | 25 ------------ pkg/types/ssotypes/saml.go | 3 +- ...tadata_store_stub.go => metadata_store.go} | 2 +- 39 files changed, 168 insertions(+), 211 deletions(-) rename pkg/types/telemetrytypes/telemetrytypestest/{metadata_store_stub.go => metadata_store.go} (99%) diff --git a/.golangci.yml b/.golangci.yml index bd48535d94..fa16191b17 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,4 +1,33 @@ +linters: + default: standard + enable: + - bodyclose + - misspell + - nilnil + - sloglint + - depguard + - iface + +linters-settings: + sloglint: + no-mixed-args: true + kv-only: true + no-global: all + context: all + static-msg: true + msg-style: lowercased + key-naming-case: snake + depguard: + rules: + nozap: + deny: + - pkg: "go.uber.org/zap" + desc: "Do not use zap logger. Use slog instead." + iface: + enable: + - identical issues: exclude-dirs: - "pkg/query-service" - "ee/query-service" + - "scripts/" diff --git a/ee/http/middleware/api_key.go b/ee/http/middleware/api_key.go index 96e35619a0..01e1981bd7 100644 --- a/ee/http/middleware/api_key.go +++ b/ee/http/middleware/api_key.go @@ -1,23 +1,24 @@ package middleware import ( + "log/slog" "net/http" "time" "github.com/SigNoz/signoz/pkg/sqlstore" "github.com/SigNoz/signoz/pkg/types" "github.com/SigNoz/signoz/pkg/types/authtypes" - "go.uber.org/zap" ) type APIKey struct { store sqlstore.SQLStore uuid *authtypes.UUID headers []string + logger *slog.Logger } -func NewAPIKey(store sqlstore.SQLStore, headers []string) *APIKey { - return &APIKey{store: store, uuid: authtypes.NewUUID(), headers: headers} +func NewAPIKey(store sqlstore.SQLStore, headers []string, logger *slog.Logger) *APIKey { + return &APIKey{store: store, uuid: authtypes.NewUUID(), headers: headers, logger: logger} } func (a *APIKey) Wrap(next http.Handler) http.Handler { @@ -77,7 +78,7 @@ func (a *APIKey) Wrap(next http.Handler) http.Handler { apiKey.LastUsed = time.Now() _, err = a.store.BunDB().NewUpdate().Model(&apiKey).Column("last_used").Where("token = ?", apiKeyToken).Where("revoked = false").Exec(r.Context()) if err != nil { - zap.L().Error("Failed to update APIKey last used in db", zap.Error(err)) + a.logger.ErrorContext(r.Context(), "failed to update last used of api key", "error", err) } }) diff --git a/ee/licensing/httplicensing/provider.go b/ee/licensing/httplicensing/provider.go index 3764edc7e9..0c63ff295b 100644 --- a/ee/licensing/httplicensing/provider.go +++ b/ee/licensing/httplicensing/provider.go @@ -79,7 +79,6 @@ func (provider *provider) Validate(ctx context.Context) error { } if len(organizations) == 0 { - provider.settings.Logger().DebugContext(ctx, "no organizations found, defaulting to basic plan") err = provider.InitFeatures(ctx, licensetypes.BasicPlan) if err != nil { return err @@ -129,15 +128,14 @@ func (provider *provider) GetActive(ctx context.Context, organizationID valuer.U } func (provider *provider) Refresh(ctx context.Context, organizationID valuer.UUID) error { - provider.settings.Logger().DebugContext(ctx, "license validation started for organizationID", "organizationID", organizationID.StringValue()) activeLicense, err := provider.GetActive(ctx, organizationID) if err != nil && !errors.Ast(err, errors.TypeNotFound) { - provider.settings.Logger().ErrorContext(ctx, "license validation failed", "organizationID", organizationID.StringValue()) + provider.settings.Logger().ErrorContext(ctx, "license validation failed", "org_id", organizationID.StringValue()) return err } if err != nil && errors.Ast(err, errors.TypeNotFound) { - provider.settings.Logger().DebugContext(ctx, "no active license found, defaulting to basic plan", "organizationID", organizationID.StringValue()) + provider.settings.Logger().DebugContext(ctx, "no active license found, defaulting to basic plan", "org_id", organizationID.StringValue()) err = provider.InitFeatures(ctx, licensetypes.BasicPlan) if err != nil { return err @@ -147,10 +145,8 @@ func (provider *provider) Refresh(ctx context.Context, organizationID valuer.UUI data, err := provider.zeus.GetLicense(ctx, activeLicense.Key) if err != nil { - provider.settings.Logger().ErrorContext(ctx, "failed to validate the license with upstream server", "licenseID", activeLicense.Key, "organizationID", organizationID.StringValue()) - if time.Since(activeLicense.LastValidatedAt) > time.Duration(provider.config.FailureThreshold)*provider.config.PollInterval { - provider.settings.Logger().ErrorContext(ctx, "license validation failed for consecutive poll intervals. defaulting to basic plan", "failureThreshold", provider.config.FailureThreshold, "licenseID", activeLicense.ID.StringValue(), "organizationID", organizationID.StringValue()) + provider.settings.Logger().ErrorContext(ctx, "license validation failed for consecutive poll intervals, defaulting to basic plan", "failure_threshold", provider.config.FailureThreshold, "license_id", activeLicense.ID.StringValue(), "org_id", organizationID.StringValue()) err = provider.InitFeatures(ctx, licensetypes.BasicPlan) if err != nil { return err @@ -165,7 +161,6 @@ func (provider *provider) Refresh(ctx context.Context, organizationID valuer.UUI return errors.Wrapf(err, errors.TypeInternal, errors.CodeInternal, "failed to create license entity from license data") } - provider.settings.Logger().DebugContext(ctx, "license validation completed successfully", "licenseID", activeLicense.ID, "organizationID", organizationID.StringValue()) updatedStorableLicense := licensetypes.NewStorableLicenseFromLicense(activeLicense) err = provider.store.Update(ctx, organizationID, updatedStorableLicense) if err != nil { diff --git a/ee/modules/user/impluser/module.go b/ee/modules/user/impluser/module.go index 92200f2aa3..c62526a4b2 100644 --- a/ee/modules/user/impluser/module.go +++ b/ee/modules/user/impluser/module.go @@ -15,7 +15,6 @@ import ( "github.com/SigNoz/signoz/pkg/types" "github.com/SigNoz/signoz/pkg/types/authtypes" "github.com/SigNoz/signoz/pkg/valuer" - "go.uber.org/zap" ) // EnterpriseModule embeds the base module implementation @@ -67,7 +66,6 @@ func (m *Module) createUserForSAMLRequest(ctx context.Context, email string) (*t func (m *Module) PrepareSsoRedirect(ctx context.Context, redirectUri, email string, jwt *authtypes.JWT) (string, error) { users, err := m.GetUsersByEmail(ctx, email) if err != nil { - zap.L().Error("failed to get user with email received from auth provider", zap.String("error", err.Error())) return "", err } user := &types.User{} @@ -76,7 +74,6 @@ func (m *Module) PrepareSsoRedirect(ctx context.Context, redirectUri, email stri newUser, err := m.createUserForSAMLRequest(ctx, email) user = newUser if err != nil { - zap.L().Error("failed to create user with email received from auth provider", zap.Error(err)) return "", err } } else { @@ -85,7 +82,6 @@ func (m *Module) PrepareSsoRedirect(ctx context.Context, redirectUri, email stri tokenStore, err := m.GetJWTForUser(ctx, user) if err != nil { - zap.L().Error("failed to generate token for SSO login user", zap.Error(err)) return "", err } @@ -164,7 +160,6 @@ func (m *Module) LoginPrecheck(ctx context.Context, orgID, email, sourceUrl stri // the EE handler wrapper passes the feature flag value in context ssoAvailable, ok := ctx.Value(types.SSOAvailable).(bool) if !ok { - zap.L().Error("failed to retrieve ssoAvailable from context") return nil, errors.New(errors.TypeInternal, errors.CodeInternal, "failed to retrieve SSO availability") } @@ -197,7 +192,6 @@ func (m *Module) LoginPrecheck(ctx context.Context, orgID, email, sourceUrl stri // the front-end will redirect user to this url resp.SSOUrl, err = orgDomain.BuildSsoUrl(siteUrl) if err != nil { - zap.L().Error("failed to prepare saml request for domain", zap.String("domain", orgDomain.Name), zap.Error(err)) return nil, errors.New(errors.TypeInternal, errors.CodeInternal, "failed to prepare saml request for domain") } diff --git a/ee/query-service/app/api/api.go b/ee/query-service/app/api/api.go index 3d57291561..2e512823f5 100644 --- a/ee/query-service/app/api/api.go +++ b/ee/query-service/app/api/api.go @@ -67,7 +67,7 @@ func NewAPIHandler(opts APIHandlerOptions, signoz *signoz.SigNoz) (*APIHandler, FluxInterval: opts.FluxInterval, AlertmanagerAPI: alertmanager.NewAPI(signoz.Alertmanager), LicensingAPI: httplicensing.NewLicensingAPI(signoz.Licensing), - FieldsAPI: fields.NewAPI(signoz.TelemetryStore), + FieldsAPI: fields.NewAPI(signoz.TelemetryStore, signoz.Instrumentation.Logger()), Signoz: signoz, }) diff --git a/ee/query-service/app/server.go b/ee/query-service/app/server.go index 9411c05e90..f694297b8c 100644 --- a/ee/query-service/app/server.go +++ b/ee/query-service/app/server.go @@ -247,15 +247,15 @@ func (s *Server) createPrivateServer(apiHandler *api.APIHandler) (*http.Server, r := baseapp.NewRouter() - r.Use(middleware.NewAuth(zap.L(), s.serverOptions.Jwt, []string{"Authorization", "Sec-WebSocket-Protocol"}).Wrap) - r.Use(eemiddleware.NewAPIKey(s.serverOptions.SigNoz.SQLStore, []string{"SIGNOZ-API-KEY"}).Wrap) - r.Use(middleware.NewTimeout(zap.L(), + r.Use(middleware.NewAuth(s.serverOptions.Jwt, []string{"Authorization", "Sec-WebSocket-Protocol"}).Wrap) + r.Use(eemiddleware.NewAPIKey(s.serverOptions.SigNoz.SQLStore, []string{"SIGNOZ-API-KEY"}, s.serverOptions.SigNoz.Instrumentation.Logger()).Wrap) + r.Use(middleware.NewTimeout(s.serverOptions.SigNoz.Instrumentation.Logger(), s.serverOptions.Config.APIServer.Timeout.ExcludedRoutes, s.serverOptions.Config.APIServer.Timeout.Default, s.serverOptions.Config.APIServer.Timeout.Max, ).Wrap) - r.Use(middleware.NewAnalytics(zap.L()).Wrap) - r.Use(middleware.NewLogging(zap.L(), s.serverOptions.Config.APIServer.Logging.ExcludedRoutes).Wrap) + r.Use(middleware.NewAnalytics().Wrap) + r.Use(middleware.NewLogging(s.serverOptions.SigNoz.Instrumentation.Logger(), s.serverOptions.Config.APIServer.Logging.ExcludedRoutes).Wrap) apiHandler.RegisterPrivateRoutes(r) @@ -279,15 +279,15 @@ func (s *Server) createPublicServer(apiHandler *api.APIHandler, web web.Web) (*h r := baseapp.NewRouter() am := middleware.NewAuthZ(s.serverOptions.SigNoz.Instrumentation.Logger()) - r.Use(middleware.NewAuth(zap.L(), s.serverOptions.Jwt, []string{"Authorization", "Sec-WebSocket-Protocol"}).Wrap) - r.Use(eemiddleware.NewAPIKey(s.serverOptions.SigNoz.SQLStore, []string{"SIGNOZ-API-KEY"}).Wrap) - r.Use(middleware.NewTimeout(zap.L(), + r.Use(middleware.NewAuth(s.serverOptions.Jwt, []string{"Authorization", "Sec-WebSocket-Protocol"}).Wrap) + r.Use(eemiddleware.NewAPIKey(s.serverOptions.SigNoz.SQLStore, []string{"SIGNOZ-API-KEY"}, s.serverOptions.SigNoz.Instrumentation.Logger()).Wrap) + r.Use(middleware.NewTimeout(s.serverOptions.SigNoz.Instrumentation.Logger(), s.serverOptions.Config.APIServer.Timeout.ExcludedRoutes, s.serverOptions.Config.APIServer.Timeout.Default, s.serverOptions.Config.APIServer.Timeout.Max, ).Wrap) - r.Use(middleware.NewAnalytics(zap.L()).Wrap) - r.Use(middleware.NewLogging(zap.L(), s.serverOptions.Config.APIServer.Logging.ExcludedRoutes).Wrap) + r.Use(middleware.NewAnalytics().Wrap) + r.Use(middleware.NewLogging(s.serverOptions.SigNoz.Instrumentation.Logger(), s.serverOptions.Config.APIServer.Logging.ExcludedRoutes).Wrap) apiHandler.RegisterRoutes(r, am) apiHandler.RegisterLogsRoutes(r, am) diff --git a/pkg/alertmanager/legacyalertmanager/provider.go b/pkg/alertmanager/legacyalertmanager/provider.go index f61d8c3720..b8fbba68e3 100644 --- a/pkg/alertmanager/legacyalertmanager/provider.go +++ b/pkg/alertmanager/legacyalertmanager/provider.go @@ -168,7 +168,7 @@ func (provider *provider) putAlerts(ctx context.Context, orgID string, alerts al receivers := cfg.ReceiverNamesFromRuleID(ruleID) if len(receivers) == 0 { - provider.settings.Logger().WarnContext(ctx, "cannot find receivers for alert, skipping sending alert to alertmanager", "ruleID", ruleID, "alert", alert) + provider.settings.Logger().WarnContext(ctx, "cannot find receivers for alert, skipping sending alert to alertmanager", "rule_id", ruleID, "alert", alert) continue } diff --git a/pkg/alertmanager/service.go b/pkg/alertmanager/service.go index 8106b678d3..d8fdd74b28 100644 --- a/pkg/alertmanager/service.go +++ b/pkg/alertmanager/service.go @@ -53,7 +53,7 @@ func (service *Service) SyncServers(ctx context.Context) error { for _, orgID := range orgIDs { config, err := service.getConfig(ctx, orgID) if err != nil { - service.settings.Logger().Error("failed to get alertmanager config for org", "orgID", orgID, "error", err) + service.settings.Logger().ErrorContext(ctx, "failed to get alertmanager config for org", "org_id", orgID, "error", err) continue } @@ -61,7 +61,7 @@ func (service *Service) SyncServers(ctx context.Context) error { if _, ok := service.servers[orgID]; !ok { server, err := service.newServer(ctx, orgID) if err != nil { - service.settings.Logger().Error("failed to create alertmanager server", "orgID", orgID, "error", err) + service.settings.Logger().ErrorContext(ctx, "failed to create alertmanager server", "org_id", orgID, "error", err) continue } @@ -69,13 +69,13 @@ func (service *Service) SyncServers(ctx context.Context) error { } if service.servers[orgID].Hash() == config.StoreableConfig().Hash { - service.settings.Logger().Debug("skipping alertmanager sync for org", "orgID", orgID, "hash", config.StoreableConfig().Hash) + service.settings.Logger().DebugContext(ctx, "skipping alertmanager sync for org", "org_id", orgID, "hash", config.StoreableConfig().Hash) continue } err = service.servers[orgID].SetConfig(ctx, config) if err != nil { - service.settings.Logger().Error("failed to set config for alertmanager server", "orgID", orgID, "error", err) + service.settings.Logger().ErrorContext(ctx, "failed to set config for alertmanager server", "org_id", orgID, "error", err) continue } } @@ -142,7 +142,7 @@ func (service *Service) Stop(ctx context.Context) error { for _, server := range service.servers { if err := server.Stop(ctx); err != nil { errs = append(errs, err) - service.settings.Logger().Error("failed to stop alertmanager server", "error", err) + service.settings.Logger().ErrorContext(ctx, "failed to stop alertmanager server", "error", err) } } @@ -167,7 +167,7 @@ func (service *Service) newServer(ctx context.Context, orgID string) (*alertmana } if beforeCompareAndSelectHash == config.StoreableConfig().Hash { - service.settings.Logger().Debug("skipping config store update for org", "orgID", orgID, "hash", config.StoreableConfig().Hash) + service.settings.Logger().DebugContext(ctx, "skipping config store update for org", "org_id", orgID, "hash", config.StoreableConfig().Hash) return server, nil } diff --git a/pkg/apis/fields/api.go b/pkg/apis/fields/api.go index e32f75ec42..2341bc6c61 100644 --- a/pkg/apis/fields/api.go +++ b/pkg/apis/fields/api.go @@ -3,6 +3,7 @@ package fields import ( "bytes" "io" + "log/slog" "net/http" "github.com/SigNoz/signoz/pkg/http/render" @@ -12,7 +13,6 @@ import ( "github.com/SigNoz/signoz/pkg/telemetrystore" "github.com/SigNoz/signoz/pkg/telemetrytraces" "github.com/SigNoz/signoz/pkg/types/telemetrytypes" - "go.uber.org/zap" ) type API struct { @@ -20,9 +20,9 @@ type API struct { telemetryMetadataStore telemetrytypes.MetadataStore } -func NewAPI(telemetryStore telemetrystore.TelemetryStore) *API { - +func NewAPI(telemetryStore telemetrystore.TelemetryStore, logger *slog.Logger) *API { telemetryMetadataStore := telemetrymetadata.NewTelemetryMetaStore( + logger, telemetryStore, telemetrytraces.DBName, telemetrytraces.TagAttributesV2TableName, @@ -99,7 +99,6 @@ func (api *API) GetFieldsValues(w http.ResponseWriter, r *http.Request) { relatedValues, err := api.telemetryMetadataStore.GetRelatedValues(ctx, fieldValueSelector) if err != nil { // we don't want to return error if we fail to get related values for some reason - zap.L().Error("failed to get related values", zap.Error(err)) relatedValues = []string{} } diff --git a/pkg/cache/memorycache/provider.go b/pkg/cache/memorycache/provider.go index 2ff40e8266..7a45bcfa03 100644 --- a/pkg/cache/memorycache/provider.go +++ b/pkg/cache/memorycache/provider.go @@ -37,7 +37,7 @@ func (provider *provider) Set(ctx context.Context, orgID valuer.UUID, cacheKey s } if ttl == 0 { - provider.settings.Logger().WarnContext(ctx, "zero value for TTL found. defaulting to the base TTL", "cacheKey", cacheKey, "defaultTTL", provider.config.Memory.TTL) + provider.settings.Logger().WarnContext(ctx, "zero value for TTL found. defaulting to the base TTL", "cache_key", cacheKey, "default_ttl", provider.config.Memory.TTL) } provider.cc.Set(strings.Join([]string{orgID.StringValue(), cacheKey}, "::"), data, ttl) return nil diff --git a/pkg/cache/rediscache/provider.go b/pkg/cache/rediscache/provider.go index 0a43b4bb19..8106628500 100644 --- a/pkg/cache/rediscache/provider.go +++ b/pkg/cache/rediscache/provider.go @@ -14,34 +14,30 @@ import ( "github.com/SigNoz/signoz/pkg/types/cachetypes" "github.com/SigNoz/signoz/pkg/valuer" "github.com/go-redis/redis/v8" - "go.uber.org/zap" ) type provider struct { - client *redis.Client + client *redis.Client + settings factory.ScopedProviderSettings } func NewFactory() factory.ProviderFactory[cache.Cache, cache.Config] { return factory.NewProviderFactory(factory.MustNewName("redis"), New) } -func New(ctx context.Context, settings factory.ProviderSettings, config cache.Config) (cache.Cache, error) { - provider := new(provider) - provider.client = redis.NewClient(&redis.Options{ +func New(ctx context.Context, providerSettings factory.ProviderSettings, config cache.Config) (cache.Cache, error) { + settings := factory.NewScopedProviderSettings(providerSettings, "github.com/SigNoz/signoz/pkg/cache/rediscache") + client := redis.NewClient(&redis.Options{ Addr: strings.Join([]string{config.Redis.Host, fmt.Sprint(config.Redis.Port)}, ":"), Password: config.Redis.Password, DB: config.Redis.DB, }) - if err := provider.client.Ping(ctx).Err(); err != nil { + if err := client.Ping(ctx).Err(); err != nil { return nil, err } - return provider, nil -} - -func WithClient(client *redis.Client) *provider { - return &provider{client: client} + return &provider{client: client, settings: settings}, nil } func (c *provider) Set(ctx context.Context, orgID valuer.UUID, cacheKey string, data cachetypes.Cacheable, ttl time.Duration) error { @@ -70,6 +66,6 @@ func (c *provider) DeleteMany(ctx context.Context, orgID valuer.UUID, cacheKeys } if err := c.client.Del(ctx, updatedCacheKeys...).Err(); err != nil { - zap.L().Error("error deleting cache keys", zap.Strings("cacheKeys", cacheKeys), zap.Error(err)) + c.settings.Logger().ErrorContext(ctx, "error deleting cache keys", "cache_keys", cacheKeys, "error", err) } } diff --git a/pkg/cache/rediscache/provider_test.go b/pkg/cache/rediscache/provider_test.go index 223a41aeb8..e9a6ff2a3a 100644 --- a/pkg/cache/rediscache/provider_test.go +++ b/pkg/cache/rediscache/provider_test.go @@ -7,6 +7,8 @@ import ( "testing" "time" + "github.com/SigNoz/signoz/pkg/factory" + "github.com/SigNoz/signoz/pkg/factory/factorytest" "github.com/SigNoz/signoz/pkg/valuer" "github.com/go-redis/redismock/v8" "github.com/stretchr/testify/assert" @@ -28,7 +30,7 @@ func (ce *CacheableEntity) UnmarshalBinary(data []byte) error { func TestSet(t *testing.T) { db, mock := redismock.NewClientMock() - cache := WithClient(db) + cache := &provider{client: db, settings: factory.NewScopedProviderSettings(factorytest.NewSettings(), "github.com/SigNoz/signoz/pkg/cache/rediscache")} storeCacheableEntity := &CacheableEntity{ Key: "some-random-key", Value: 1, @@ -46,7 +48,7 @@ func TestSet(t *testing.T) { func TestGet(t *testing.T) { db, mock := redismock.NewClientMock() - cache := WithClient(db) + cache := &provider{client: db, settings: factory.NewScopedProviderSettings(factorytest.NewSettings(), "github.com/SigNoz/signoz/pkg/cache/rediscache")} storeCacheableEntity := &CacheableEntity{ Key: "some-random-key", Value: 1, @@ -75,7 +77,7 @@ func TestGet(t *testing.T) { func TestDelete(t *testing.T) { db, mock := redismock.NewClientMock() - c := WithClient(db) + cache := &provider{client: db, settings: factory.NewScopedProviderSettings(factorytest.NewSettings(), "github.com/SigNoz/signoz/pkg/cache/rediscache")} storeCacheableEntity := &CacheableEntity{ Key: "some-random-key", Value: 1, @@ -84,10 +86,10 @@ func TestDelete(t *testing.T) { orgID := valuer.GenerateUUID() mock.ExpectSet(strings.Join([]string{orgID.StringValue(), "key"}, "::"), storeCacheableEntity, 10*time.Second).RedisNil() - _ = c.Set(context.Background(), orgID, "key", storeCacheableEntity, 10*time.Second) + _ = cache.Set(context.Background(), orgID, "key", storeCacheableEntity, 10*time.Second) mock.ExpectDel(strings.Join([]string{orgID.StringValue(), "key"}, "::")).RedisNil() - c.Delete(context.Background(), orgID, "key") + cache.Delete(context.Background(), orgID, "key") if err := mock.ExpectationsWereMet(); err != nil { t.Errorf("there were unfulfilled expectations: %s", err) @@ -96,7 +98,7 @@ func TestDelete(t *testing.T) { func TestDeleteMany(t *testing.T) { db, mock := redismock.NewClientMock() - c := WithClient(db) + cache := &provider{client: db, settings: factory.NewScopedProviderSettings(factorytest.NewSettings(), "github.com/SigNoz/signoz/pkg/cache/rediscache")} storeCacheableEntity := &CacheableEntity{ Key: "some-random-key", Value: 1, @@ -105,13 +107,13 @@ func TestDeleteMany(t *testing.T) { orgID := valuer.GenerateUUID() mock.ExpectSet(strings.Join([]string{orgID.StringValue(), "key"}, "::"), storeCacheableEntity, 10*time.Second).RedisNil() - _ = c.Set(context.Background(), orgID, "key", storeCacheableEntity, 10*time.Second) + _ = cache.Set(context.Background(), orgID, "key", storeCacheableEntity, 10*time.Second) mock.ExpectSet(strings.Join([]string{orgID.StringValue(), "key2"}, "::"), storeCacheableEntity, 10*time.Second).RedisNil() - _ = c.Set(context.Background(), orgID, "key2", storeCacheableEntity, 10*time.Second) + _ = cache.Set(context.Background(), orgID, "key2", storeCacheableEntity, 10*time.Second) mock.ExpectDel(strings.Join([]string{orgID.StringValue(), "key"}, "::"), strings.Join([]string{orgID.StringValue(), "key2"}, "::")).RedisNil() - c.DeleteMany(context.Background(), orgID, []string{"key", "key2"}) + cache.DeleteMany(context.Background(), orgID, []string{"key", "key2"}) if err := mock.ExpectationsWereMet(); err != nil { t.Errorf("there were unfulfilled expectations: %s", err) diff --git a/pkg/emailing/smtpemailing/provider.go b/pkg/emailing/smtpemailing/provider.go index 66b9489a11..07eb0c6100 100644 --- a/pkg/emailing/smtpemailing/provider.go +++ b/pkg/emailing/smtpemailing/provider.go @@ -25,7 +25,7 @@ func New(ctx context.Context, providerSettings factory.ProviderSettings, config settings := factory.NewScopedProviderSettings(providerSettings, "github.com/SigNoz/signoz/pkg/emailing/smtpemailing") // Try to create a template store. If it fails, use an empty store. - store, err := filetemplatestore.NewStore(config.Templates.Directory, emailtypes.Templates, settings.Logger()) + store, err := filetemplatestore.NewStore(ctx, config.Templates.Directory, emailtypes.Templates, settings.Logger()) if err != nil { settings.Logger().ErrorContext(ctx, "failed to create template store, using empty store", "error", err) store = filetemplatestore.NewEmptyStore() diff --git a/pkg/emailing/templatestore/filetemplatestore/store.go b/pkg/emailing/templatestore/filetemplatestore/store.go index 5af518d0f8..d79a249247 100644 --- a/pkg/emailing/templatestore/filetemplatestore/store.go +++ b/pkg/emailing/templatestore/filetemplatestore/store.go @@ -21,7 +21,7 @@ type store struct { fs map[emailtypes.TemplateName]*template.Template } -func NewStore(baseDir string, templates []emailtypes.TemplateName, logger *slog.Logger) (emailtypes.TemplateStore, error) { +func NewStore(ctx context.Context, baseDir string, templates []emailtypes.TemplateName, logger *slog.Logger) (emailtypes.TemplateStore, error) { fs := make(map[emailtypes.TemplateName]*template.Template) fis, err := os.ReadDir(filepath.Clean(baseDir)) if err != nil { @@ -45,7 +45,7 @@ func NewStore(baseDir string, templates []emailtypes.TemplateName, logger *slog. t, err := parseTemplateFile(filepath.Join(baseDir, fi.Name()), templateName) if err != nil { - logger.Error("failed to parse template file", "template", templateName, "path", filepath.Join(baseDir, fi.Name()), "error", err) + logger.ErrorContext(ctx, "failed to parse template file", "template", templateName, "path", filepath.Join(baseDir, fi.Name()), "error", err) continue } @@ -54,7 +54,7 @@ func NewStore(baseDir string, templates []emailtypes.TemplateName, logger *slog. } if err := checkMissingTemplates(templates, foundTemplates); err != nil { - logger.Error("some templates are missing", "error", err) + logger.ErrorContext(ctx, "some templates are missing", "error", err) } return &store{fs: fs}, nil diff --git a/pkg/http/middleware/analytics.go b/pkg/http/middleware/analytics.go index 0930935bbc..db8d871b12 100644 --- a/pkg/http/middleware/analytics.go +++ b/pkg/http/middleware/analytics.go @@ -11,19 +11,12 @@ import ( "github.com/SigNoz/signoz/pkg/query-service/telemetry" "github.com/SigNoz/signoz/pkg/types/authtypes" "github.com/gorilla/mux" - "go.uber.org/zap" ) -type Analytics struct { - logger *zap.Logger -} +type Analytics struct{} -func NewAnalytics(logger *zap.Logger) *Analytics { - if logger == nil { - panic("cannot build analytics middleware, logger is empty") - } - - return &Analytics{logger: logger} +func NewAnalytics() *Analytics { + return &Analytics{} } func (a *Analytics) Wrap(next http.Handler) http.Handler { @@ -94,22 +87,10 @@ func (a *Analytics) extractQueryRangeData(path string, r *http.Request) (map[str referrer := r.Header.Get("Referer") - dashboardMatched, err := regexp.MatchString(`/dashboard/[a-zA-Z0-9\-]+/(new|edit)(?:\?.*)?$`, referrer) - if err != nil { - a.logger.Error("error while matching the referrer", zap.Error(err)) - } - alertMatched, err := regexp.MatchString(`/alerts/(new|edit)(?:\?.*)?$`, referrer) - if err != nil { - a.logger.Error("error while matching the alert: ", zap.Error(err)) - } - logsExplorerMatched, err := regexp.MatchString(`/logs/logs-explorer(?:\?.*)?$`, referrer) - if err != nil { - a.logger.Error("error while matching the logs explorer: ", zap.Error(err)) - } - traceExplorerMatched, err := regexp.MatchString(`/traces-explorer(?:\?.*)?$`, referrer) - if err != nil { - a.logger.Error("error while matching the trace explorer: ", zap.Error(err)) - } + dashboardMatched, _ := regexp.MatchString(`/dashboard/[a-zA-Z0-9\-]+/(new|edit)(?:\?.*)?$`, referrer) + alertMatched, _ := regexp.MatchString(`/alerts/(new|edit)(?:\?.*)?$`, referrer) + logsExplorerMatched, _ := regexp.MatchString(`/logs/logs-explorer(?:\?.*)?$`, referrer) + traceExplorerMatched, _ := regexp.MatchString(`/traces-explorer(?:\?.*)?$`, referrer) queryInfoResult := telemetry.GetInstance().CheckQueryInfo(postData) diff --git a/pkg/http/middleware/auth.go b/pkg/http/middleware/auth.go index 719d66bdf1..491ccb93f1 100644 --- a/pkg/http/middleware/auth.go +++ b/pkg/http/middleware/auth.go @@ -4,21 +4,15 @@ import ( "net/http" "github.com/SigNoz/signoz/pkg/types/authtypes" - "go.uber.org/zap" ) type Auth struct { - logger *zap.Logger jwt *authtypes.JWT headers []string } -func NewAuth(logger *zap.Logger, jwt *authtypes.JWT, headers []string) *Auth { - if logger == nil { - panic("cannot build auth middleware, logger is empty") - } - - return &Auth{logger: logger, jwt: jwt, headers: headers} +func NewAuth(jwt *authtypes.JWT, headers []string) *Auth { + return &Auth{jwt: jwt, headers: headers} } func (a *Auth) Wrap(next http.Handler) http.Handler { diff --git a/pkg/http/middleware/cache_test.go b/pkg/http/middleware/cache_test.go index 80c55a767d..3adeee06bb 100644 --- a/pkg/http/middleware/cache_test.go +++ b/pkg/http/middleware/cache_test.go @@ -47,6 +47,9 @@ func TestCache(t *testing.T) { res, err := http.DefaultClient.Do(req) require.NoError(t, err) + defer func() { + require.NoError(t, res.Body.Close()) + }() actual := res.Header.Get("Cache-control") require.NoError(t, err) diff --git a/pkg/http/middleware/logging.go b/pkg/http/middleware/logging.go index 61dbbab67d..ba3d805758 100644 --- a/pkg/http/middleware/logging.go +++ b/pkg/http/middleware/logging.go @@ -3,6 +3,7 @@ package middleware import ( "bytes" "context" + "log/slog" "net" "net/http" "net/url" @@ -13,7 +14,6 @@ import ( "github.com/SigNoz/signoz/pkg/types/authtypes" "github.com/gorilla/mux" semconv "go.opentelemetry.io/otel/semconv/v1.26.0" - "go.uber.org/zap" ) const ( @@ -21,22 +21,18 @@ const ( ) type Logging struct { - logger *zap.Logger + logger *slog.Logger excludedRoutes map[string]struct{} } -func NewLogging(logger *zap.Logger, excludedRoutes []string) *Logging { - if logger == nil { - panic("cannot build logging, logger is empty") - } - +func NewLogging(logger *slog.Logger, excludedRoutes []string) *Logging { excludedRoutesMap := make(map[string]struct{}) for _, route := range excludedRoutes { excludedRoutesMap[route] = struct{}{} } return &Logging{ - logger: logger.Named(pkgname), + logger: logger.With("pkg", pkgname), excludedRoutes: excludedRoutesMap, } } @@ -50,13 +46,13 @@ func (middleware *Logging) Wrap(next http.Handler) http.Handler { path = req.URL.Path } - fields := []zap.Field{ - zap.String(string(semconv.ClientAddressKey), req.RemoteAddr), - zap.String(string(semconv.UserAgentOriginalKey), req.UserAgent()), - zap.String(string(semconv.ServerAddressKey), host), - zap.String(string(semconv.ServerPortKey), port), - zap.Int64(string(semconv.HTTPRequestSizeKey), req.ContentLength), - zap.String(string(semconv.HTTPRouteKey), path), + fields := []any{ + string(semconv.ClientAddressKey), req.RemoteAddr, + string(semconv.UserAgentOriginalKey), req.UserAgent(), + string(semconv.ServerAddressKey), host, + string(semconv.ServerPortKey), port, + string(semconv.HTTPRequestSizeKey), req.ContentLength, + string(semconv.HTTPRouteKey), path, } logCommentKVs := middleware.getLogCommentKVs(req) @@ -73,19 +69,19 @@ func (middleware *Logging) Wrap(next http.Handler) http.Handler { statusCode, err := writer.StatusCode(), writer.WriteError() fields = append(fields, - zap.Int(string(semconv.HTTPResponseStatusCodeKey), statusCode), - zap.Duration(string(semconv.HTTPServerRequestDurationName), time.Since(start)), + string(semconv.HTTPResponseStatusCodeKey), statusCode, + string(semconv.HTTPServerRequestDurationName), time.Since(start), ) if err != nil { - fields = append(fields, zap.Error(err)) - middleware.logger.Error(logMessage, fields...) + fields = append(fields, "error", err) + middleware.logger.ErrorContext(req.Context(), logMessage, fields...) } else { // when the status code is 400 or >=500, and the response body is not empty. if badResponseBuffer.Len() != 0 { - fields = append(fields, zap.String("response.body", badResponseBuffer.String())) + fields = append(fields, "response.body", badResponseBuffer.String()) } - middleware.logger.Info(logMessage, fields...) + middleware.logger.InfoContext(req.Context(), logMessage, fields...) } }) } diff --git a/pkg/http/middleware/timeout.go b/pkg/http/middleware/timeout.go index 84ca3d27b6..9909336be7 100644 --- a/pkg/http/middleware/timeout.go +++ b/pkg/http/middleware/timeout.go @@ -2,11 +2,10 @@ package middleware import ( "context" + "log/slog" "net/http" "strings" "time" - - "go.uber.org/zap" ) const ( @@ -14,7 +13,7 @@ const ( ) type Timeout struct { - logger *zap.Logger + logger *slog.Logger excluded map[string]struct{} // The default timeout defaultTimeout time.Duration @@ -22,11 +21,7 @@ type Timeout struct { maxTimeout time.Duration } -func NewTimeout(logger *zap.Logger, excludedRoutes []string, defaultTimeout time.Duration, maxTimeout time.Duration) *Timeout { - if logger == nil { - panic("cannot build timeout, logger is empty") - } - +func NewTimeout(logger *slog.Logger, excludedRoutes []string, defaultTimeout time.Duration, maxTimeout time.Duration) *Timeout { excluded := make(map[string]struct{}, len(excludedRoutes)) for _, route := range excludedRoutes { excluded[route] = struct{}{} @@ -41,7 +36,7 @@ func NewTimeout(logger *zap.Logger, excludedRoutes []string, defaultTimeout time } return &Timeout{ - logger: logger.Named(pkgname), + logger: logger.With("pkg", pkgname), excluded: excluded, defaultTimeout: defaultTimeout, maxTimeout: maxTimeout, @@ -56,7 +51,7 @@ func (middleware *Timeout) Wrap(next http.Handler) http.Handler { if incoming != "" { parsed, err := time.ParseDuration(strings.TrimSpace(incoming) + "s") if err != nil { - middleware.logger.Warn("cannot parse timeout in header, using default timeout", zap.String("timeout", incoming), zap.Error(err), zap.Any("context", req.Context())) + middleware.logger.WarnContext(req.Context(), "cannot parse timeout in header, using default timeout", "timeout", incoming, "error", err) } else { if parsed > middleware.maxTimeout { actual = middleware.maxTimeout diff --git a/pkg/http/middleware/timeout_test.go b/pkg/http/middleware/timeout_test.go index e18291786d..56eb687b15 100644 --- a/pkg/http/middleware/timeout_test.go +++ b/pkg/http/middleware/timeout_test.go @@ -1,13 +1,14 @@ package middleware import ( + "io" + "log/slog" "net" "net/http" "testing" "time" "github.com/stretchr/testify/require" - "go.uber.org/zap" ) func TestTimeout(t *testing.T) { @@ -16,7 +17,7 @@ func TestTimeout(t *testing.T) { writeTimeout := 6 * time.Second defaultTimeout := 2 * time.Second maxTimeout := 4 * time.Second - m := NewTimeout(zap.NewNop(), []string{"/excluded"}, defaultTimeout, maxTimeout) + m := NewTimeout(slog.New(slog.NewTextHandler(io.Discard, nil)), []string{"/excluded"}, defaultTimeout, maxTimeout) listener, err := net.Listen("tcp", "localhost:0") require.NoError(t, err) @@ -70,8 +71,11 @@ func TestTimeout(t *testing.T) { require.NoError(t, err) req.Header.Add(headerName, tc.header) - _, err = http.DefaultClient.Do(req) + res, err := http.DefaultClient.Do(req) require.NoError(t, err) + defer func() { + require.NoError(t, res.Body.Close()) + }() // confirm that we waited at least till the "wait" time require.GreaterOrEqual(t, time.Since(start), tc.wait) diff --git a/pkg/http/render/render_test.go b/pkg/http/render/render_test.go index 5b6b28149c..42f4565de7 100644 --- a/pkg/http/render/render_test.go +++ b/pkg/http/render/render_test.go @@ -47,6 +47,9 @@ func TestSuccess(t *testing.T) { res, err := http.DefaultClient.Do(req) require.NoError(t, err) + defer func() { + require.NoError(t, res.Body.Close()) + }() actual, err := io.ReadAll(res.Body) require.NoError(t, err) @@ -104,6 +107,9 @@ func TestError(t *testing.T) { res, err := http.DefaultClient.Do(req) require.NoError(t, err) + defer func() { + require.NoError(t, res.Body.Close()) + }() actual, err := io.ReadAll(res.Body) require.NoError(t, err) diff --git a/pkg/http/server/server.go b/pkg/http/server/server.go index 449eff28f8..6d1c5c71a6 100644 --- a/pkg/http/server/server.go +++ b/pkg/http/server/server.go @@ -3,23 +3,23 @@ package server import ( "context" "fmt" + "log/slog" "net/http" "time" "github.com/SigNoz/signoz/pkg/factory" - "go.uber.org/zap" ) var _ factory.Service = (*Server)(nil) type Server struct { srv *http.Server - logger *zap.Logger + logger *slog.Logger handler http.Handler cfg Config } -func New(logger *zap.Logger, cfg Config, handler http.Handler) (*Server, error) { +func New(logger *slog.Logger, cfg Config, handler http.Handler) (*Server, error) { if handler == nil { return nil, fmt.Errorf("cannot build http server, handler is required") } @@ -38,17 +38,17 @@ func New(logger *zap.Logger, cfg Config, handler http.Handler) (*Server, error) return &Server{ srv: srv, - logger: logger.Named("go.signoz.io/pkg/http/server"), + logger: logger.With("pkg", "go.signoz.io/pkg/http/server"), handler: handler, cfg: cfg, }, nil } func (server *Server) Start(ctx context.Context) error { - server.logger.Info("starting http server", zap.String("address", server.srv.Addr)) + server.logger.InfoContext(ctx, "starting http server", "address", server.srv.Addr) if err := server.srv.ListenAndServe(); err != nil { if err != http.ErrServerClosed { - server.logger.Error("failed to start server", zap.Error(err), zap.Any("context", ctx)) + server.logger.ErrorContext(ctx, "failed to start server", "error", err) return err } } @@ -60,10 +60,10 @@ func (server *Server) Stop(ctx context.Context) error { defer cancel() if err := server.srv.Shutdown(ctx); err != nil { - server.logger.Error("failed to stop server", zap.Error(err), zap.Any("context", ctx)) + server.logger.ErrorContext(ctx, "failed to stop server", "error", err) return err } - server.logger.Info("server stopped gracefully", zap.Any("context", ctx)) + server.logger.InfoContext(ctx, "server stopped gracefully") return nil } diff --git a/pkg/query-service/app/server.go b/pkg/query-service/app/server.go index fbd35320c0..ea8ee195d6 100644 --- a/pkg/query-service/app/server.go +++ b/pkg/query-service/app/server.go @@ -147,7 +147,7 @@ func NewServer(serverOptions *ServerOptions) (*Server, error) { JWT: serverOptions.Jwt, AlertmanagerAPI: alertmanager.NewAPI(serverOptions.SigNoz.Alertmanager), LicensingAPI: nooplicensing.NewLicenseAPI(), - FieldsAPI: fields.NewAPI(serverOptions.SigNoz.TelemetryStore), + FieldsAPI: fields.NewAPI(serverOptions.SigNoz.TelemetryStore, serverOptions.SigNoz.Instrumentation.Logger()), Signoz: serverOptions.SigNoz, }) if err != nil { @@ -212,14 +212,14 @@ func (s *Server) createPrivateServer(api *APIHandler) (*http.Server, error) { r := NewRouter() - r.Use(middleware.NewAuth(zap.L(), s.serverOptions.Jwt, []string{"Authorization", "Sec-WebSocket-Protocol"}).Wrap) - r.Use(middleware.NewTimeout(zap.L(), + r.Use(middleware.NewAuth(s.serverOptions.Jwt, []string{"Authorization", "Sec-WebSocket-Protocol"}).Wrap) + r.Use(middleware.NewTimeout(s.serverOptions.SigNoz.Instrumentation.Logger(), s.serverOptions.Config.APIServer.Timeout.ExcludedRoutes, s.serverOptions.Config.APIServer.Timeout.Default, s.serverOptions.Config.APIServer.Timeout.Max, ).Wrap) - r.Use(middleware.NewAnalytics(zap.L()).Wrap) - r.Use(middleware.NewLogging(zap.L(), s.serverOptions.Config.APIServer.Logging.ExcludedRoutes).Wrap) + r.Use(middleware.NewAnalytics().Wrap) + r.Use(middleware.NewLogging(s.serverOptions.SigNoz.Instrumentation.Logger(), s.serverOptions.Config.APIServer.Logging.ExcludedRoutes).Wrap) api.RegisterPrivateRoutes(r) @@ -242,14 +242,14 @@ func (s *Server) createPrivateServer(api *APIHandler) (*http.Server, error) { func (s *Server) createPublicServer(api *APIHandler, web web.Web) (*http.Server, error) { r := NewRouter() - r.Use(middleware.NewAuth(zap.L(), s.serverOptions.Jwt, []string{"Authorization", "Sec-WebSocket-Protocol"}).Wrap) - r.Use(middleware.NewTimeout(zap.L(), + r.Use(middleware.NewAuth(s.serverOptions.Jwt, []string{"Authorization", "Sec-WebSocket-Protocol"}).Wrap) + r.Use(middleware.NewTimeout(s.serverOptions.SigNoz.Instrumentation.Logger(), s.serverOptions.Config.APIServer.Timeout.ExcludedRoutes, s.serverOptions.Config.APIServer.Timeout.Default, s.serverOptions.Config.APIServer.Timeout.Max, ).Wrap) - r.Use(middleware.NewAnalytics(zap.L()).Wrap) - r.Use(middleware.NewLogging(zap.L(), s.serverOptions.Config.APIServer.Logging.ExcludedRoutes).Wrap) + r.Use(middleware.NewAnalytics().Wrap) + r.Use(middleware.NewLogging(s.serverOptions.SigNoz.Instrumentation.Logger(), s.serverOptions.Config.APIServer.Logging.ExcludedRoutes).Wrap) am := middleware.NewAuthZ(s.serverOptions.SigNoz.Instrumentation.Logger()) diff --git a/pkg/query-service/tests/integration/filter_suggestions_test.go b/pkg/query-service/tests/integration/filter_suggestions_test.go index 92c8d5abf2..7c1a59bf32 100644 --- a/pkg/query-service/tests/integration/filter_suggestions_test.go +++ b/pkg/query-service/tests/integration/filter_suggestions_test.go @@ -325,7 +325,7 @@ func NewFilterSuggestionsTestBed(t *testing.T) *FilterSuggestionsTestBed { router := app.NewRouter() //add the jwt middleware - router.Use(middleware.NewAuth(zap.L(), jwt, []string{"Authorization", "Sec-WebSocket-Protocol"}).Wrap) + router.Use(middleware.NewAuth(jwt, []string{"Authorization", "Sec-WebSocket-Protocol"}).Wrap) am := middleware.NewAuthZ(instrumentationtest.New().Logger()) apiHandler.RegisterRoutes(router, am) apiHandler.RegisterQueryRangeV3Routes(router, am) diff --git a/pkg/query-service/tests/integration/signoz_cloud_integrations_test.go b/pkg/query-service/tests/integration/signoz_cloud_integrations_test.go index 934f45ce8e..f8c78f7635 100644 --- a/pkg/query-service/tests/integration/signoz_cloud_integrations_test.go +++ b/pkg/query-service/tests/integration/signoz_cloud_integrations_test.go @@ -28,7 +28,6 @@ import ( "github.com/google/uuid" mockhouse "github.com/srikanthccv/ClickHouse-go-mock" "github.com/stretchr/testify/require" - "go.uber.org/zap" ) func TestAWSIntegrationAccountLifecycle(t *testing.T) { @@ -388,7 +387,7 @@ func NewCloudIntegrationsTestBed(t *testing.T, testDB sqlstore.SQLStore) *CloudI } router := app.NewRouter() - router.Use(middleware.NewAuth(zap.L(), jwt, []string{"Authorization", "Sec-WebSocket-Protocol"}).Wrap) + router.Use(middleware.NewAuth(jwt, []string{"Authorization", "Sec-WebSocket-Protocol"}).Wrap) am := middleware.NewAuthZ(instrumentationtest.New().Logger()) apiHandler.RegisterRoutes(router, am) apiHandler.RegisterCloudIntegrationsRoutes(router, am) diff --git a/pkg/query-service/tests/integration/signoz_integrations_test.go b/pkg/query-service/tests/integration/signoz_integrations_test.go index d39098d7db..78b14cfaeb 100644 --- a/pkg/query-service/tests/integration/signoz_integrations_test.go +++ b/pkg/query-service/tests/integration/signoz_integrations_test.go @@ -30,7 +30,6 @@ import ( "github.com/SigNoz/signoz/pkg/types/pipelinetypes" mockhouse "github.com/srikanthccv/ClickHouse-go-mock" "github.com/stretchr/testify/require" - "go.uber.org/zap" ) // Higher level tests for UI facing APIs @@ -595,7 +594,7 @@ func NewIntegrationsTestBed(t *testing.T, testDB sqlstore.SQLStore) *Integration } router := app.NewRouter() - router.Use(middleware.NewAuth(zap.L(), jwt, []string{"Authorization", "Sec-WebSocket-Protocol"}).Wrap) + router.Use(middleware.NewAuth(jwt, []string{"Authorization", "Sec-WebSocket-Protocol"}).Wrap) am := middleware.NewAuthZ(instrumentationtest.New().Logger()) apiHandler.RegisterRoutes(router, am) apiHandler.RegisterIntegrationRoutes(router, am) diff --git a/pkg/ruler/rulestore/sqlrulestore/maintenance.go b/pkg/ruler/rulestore/sqlrulestore/maintenance.go index c6bb000f0a..3282af5e6a 100644 --- a/pkg/ruler/rulestore/sqlrulestore/maintenance.go +++ b/pkg/ruler/rulestore/sqlrulestore/maintenance.go @@ -9,7 +9,6 @@ import ( "github.com/SigNoz/signoz/pkg/types/authtypes" ruletypes "github.com/SigNoz/signoz/pkg/types/ruletypes" "github.com/SigNoz/signoz/pkg/valuer" - "go.uber.org/zap" ) type maintenance struct { @@ -30,7 +29,6 @@ func (r *maintenance) GetAllPlannedMaintenance(ctx context.Context, orgID string Where("org_id = ?", orgID). Scan(ctx) if err != nil { - zap.L().Error("Error in processing sql query", zap.Error(err)) return nil, err } @@ -137,7 +135,6 @@ func (r *maintenance) DeletePlannedMaintenance(ctx context.Context, id valuer.UU Where("id = ?", id.StringValue()). Exec(ctx) if err != nil { - zap.L().Error("Error in processing sql query", zap.Error(err)) return err } @@ -221,7 +218,6 @@ func (r *maintenance) EditPlannedMaintenance(ctx context.Context, maintenance ru }) if err != nil { - zap.L().Error("Error in processing sql query", zap.Error(err)) return err } diff --git a/pkg/ruler/rulestore/sqlrulestore/rule.go b/pkg/ruler/rulestore/sqlrulestore/rule.go index 4414897ede..735cce6b20 100644 --- a/pkg/ruler/rulestore/sqlrulestore/rule.go +++ b/pkg/ruler/rulestore/sqlrulestore/rule.go @@ -8,7 +8,6 @@ import ( ruletypes "github.com/SigNoz/signoz/pkg/types/ruletypes" "github.com/SigNoz/signoz/pkg/valuer" "github.com/jmoiron/sqlx" - "go.uber.org/zap" ) type rule struct { @@ -86,7 +85,6 @@ func (r *rule) GetStoredRules(ctx context.Context, orgID string) ([]*ruletypes.R Where("org_id = ?", orgID). Scan(ctx) if err != nil { - zap.L().Error("Error in processing sql query", zap.Error(err)) return rules, err } @@ -102,7 +100,6 @@ func (r *rule) GetStoredRule(ctx context.Context, id valuer.UUID) (*ruletypes.Ru Where("id = ?", id.StringValue()). Scan(ctx) if err != nil { - zap.L().Error("Error in processing sql query", zap.Error(err)) return nil, err } return rule, nil @@ -117,7 +114,6 @@ func (r *rule) GetRuleUUID(ctx context.Context, ruleID int) (*ruletypes.RuleHist Where("rule_id = ?", ruleID). Scan(ctx) if err != nil { - zap.L().Error("Error in processing sql query", zap.Error(err)) return nil, err } return ruleHistory, nil diff --git a/pkg/smtp/client/smtp.go b/pkg/smtp/client/smtp.go index 991db0c867..e3c07b4d67 100644 --- a/pkg/smtp/client/smtp.go +++ b/pkg/smtp/client/smtp.go @@ -108,7 +108,7 @@ func (c *Client) Do(ctx context.Context, tos []*mail.Address, subject string, co // Try to clean up after ourselves but don't log anything if something has failed. defer func() { if err := smtpClient.Quit(); success && err != nil { - c.logger.Warn("failed to close SMTP connection", "error", err) + c.logger.WarnContext(ctx, "failed to close SMTP connection", "error", err) } }() diff --git a/pkg/sqlmigration/026_update_integrations.go b/pkg/sqlmigration/026_update_integrations.go index 5c4cb0e41e..2611cae4dd 100644 --- a/pkg/sqlmigration/026_update_integrations.go +++ b/pkg/sqlmigration/026_update_integrations.go @@ -12,7 +12,6 @@ import ( "github.com/google/uuid" "github.com/uptrace/bun" "github.com/uptrace/bun/migrate" - "go.uber.org/zap" ) type updateIntegrations struct { @@ -332,7 +331,6 @@ func (migration *updateIntegrations) CopyOldCloudIntegrationServicesToNewCloudIn if err == sql.ErrNoRows { continue } - zap.L().Error("failed to get cloud integration id", zap.Error(err)) return nil } newServices = append(newServices, &newCloudIntegrationService{ diff --git a/pkg/sqlstore/sqlstorehook/logging.go b/pkg/sqlstore/sqlstorehook/logging.go index 08342b142a..a5ab6766f2 100644 --- a/pkg/sqlstore/sqlstorehook/logging.go +++ b/pkg/sqlstore/sqlstorehook/logging.go @@ -36,8 +36,8 @@ func (hook logging) AfterQuery(ctx context.Context, event *bun.QueryEvent) { ctx, hook.level, "::SQLSTORE-QUERY::", - "db.query.operation", event.Operation(), - "db.query.text", event.Query, - "db.duration", time.Since(event.StartTime).String(), + "db_query_operation", event.Operation(), + "db_query_text", event.Query, + "db_query_duration", time.Since(event.StartTime).String(), ) } diff --git a/pkg/telemetrylogs/filter_expr_logs_test.go b/pkg/telemetrylogs/filter_expr_logs_test.go index b98458e2ff..c667988234 100644 --- a/pkg/telemetrylogs/filter_expr_logs_test.go +++ b/pkg/telemetrylogs/filter_expr_logs_test.go @@ -459,7 +459,7 @@ func TestFilterExprLogs(t *testing.T) { expectedErrorContains: "", }, - // Conflicts with the key token, are valid and without additonal tokens, they are searched as FREETEXT + // Conflicts with the key token, are valid and without additional tokens, they are searched as FREETEXT { category: "Key token conflict", query: "status.code", diff --git a/pkg/telemetrymetadata/metadata.go b/pkg/telemetrymetadata/metadata.go index 821d6c823b..7459ebdb56 100644 --- a/pkg/telemetrymetadata/metadata.go +++ b/pkg/telemetrymetadata/metadata.go @@ -3,13 +3,13 @@ package telemetrymetadata import ( "context" "fmt" + "log/slog" "github.com/SigNoz/signoz/pkg/errors" "github.com/SigNoz/signoz/pkg/telemetrystore" qbtypes "github.com/SigNoz/signoz/pkg/types/querybuildertypes/querybuildertypesv5" "github.com/SigNoz/signoz/pkg/types/telemetrytypes" "github.com/huandu/go-sqlbuilder" - "go.uber.org/zap" ) var ( @@ -21,6 +21,7 @@ var ( ) type telemetryMetaStore struct { + logger *slog.Logger telemetrystore telemetrystore.TelemetryStore tracesDBName string tracesFieldsTblName string @@ -39,6 +40,7 @@ type telemetryMetaStore struct { } func NewTelemetryMetaStore( + logger *slog.Logger, telemetrystore telemetrystore.TelemetryStore, tracesDBName string, tracesFieldsTblName string, @@ -98,7 +100,6 @@ func (t *telemetryMetaStore) tracesTblStatementToFieldKeys(ctx context.Context) // getTracesKeys returns the keys from the spans that match the field selection criteria func (t *telemetryMetaStore) getTracesKeys(ctx context.Context, fieldKeySelectors []*telemetrytypes.FieldKeySelector) ([]*telemetrytypes.TelemetryFieldKey, error) { - if len(fieldKeySelectors) == 0 { return nil, nil } @@ -566,7 +567,7 @@ func (t *telemetryMetaStore) getRelatedValues(ctx context.Context, fieldValueSel if err == nil { sb.AddWhereClause(whereClause) } else { - zap.L().Warn("error parsing existing query for related values", zap.Error(err)) + t.logger.WarnContext(ctx, "error parsing existing query for related values", "error", err) } } @@ -586,7 +587,7 @@ func (t *telemetryMetaStore) getRelatedValues(ctx context.Context, fieldValueSel query, args := sb.BuildWithFlavor(sqlbuilder.ClickHouse) - zap.L().Debug("query for related values", zap.String("query", query), zap.Any("args", args)) + t.logger.DebugContext(ctx, "query for related values", "query", query, "args", args) rows, err := t.telemetrystore.ClickhouseDB().Query(ctx, query, args...) if err != nil { diff --git a/pkg/telemetrymetadata/metadata_test.go b/pkg/telemetrymetadata/metadata_test.go index f5d949cc33..6fc3079879 100644 --- a/pkg/telemetrymetadata/metadata_test.go +++ b/pkg/telemetrymetadata/metadata_test.go @@ -3,6 +3,8 @@ package telemetrymetadata import ( "context" "fmt" + "io" + "log/slog" "regexp" "testing" @@ -34,6 +36,7 @@ func TestGetKeys(t *testing.T) { mock := mockTelemetryStore.Mock() metadata := NewTelemetryMetaStore( + slog.New(slog.NewTextHandler(io.Discard, nil)), mockTelemetryStore, telemetrytraces.DBName, telemetrytraces.TagAttributesV2TableName, diff --git a/pkg/types/domain.go b/pkg/types/domain.go index 3134c0fe3e..1ea6eb50de 100644 --- a/pkg/types/domain.go +++ b/pkg/types/domain.go @@ -11,7 +11,6 @@ import ( "github.com/pkg/errors" saml2 "github.com/russellhaering/gosaml2" "github.com/uptrace/bun" - "go.uber.org/zap" ) type StorableOrgDomain struct { @@ -182,7 +181,6 @@ func (od *GettableOrgDomain) BuildSsoUrl(siteUrl *url.URL) (ssoUrl string, err e return googleProvider.BuildAuthURL(relayState) default: - zap.L().Error("found unsupported SSO config for the org domain", zap.String("orgDomain", od.Name)) return "", fmt.Errorf("unsupported SSO config for the domain") } diff --git a/pkg/types/integration.go b/pkg/types/integration.go index 2324c64e58..43d29c4b56 100644 --- a/pkg/types/integration.go +++ b/pkg/types/integration.go @@ -145,14 +145,12 @@ func (c *AccountConfig) Scan(src any) error { // For serializing to db func (c *AccountConfig) Value() (driver.Value, error) { if c == nil { - return nil, nil + return nil, fmt.Errorf("cloud account config is nil") } serialized, err := json.Marshal(c) if err != nil { - return nil, fmt.Errorf( - "couldn't serialize cloud account config to JSON: %w", err, - ) + return nil, fmt.Errorf("couldn't serialize cloud account config to JSON: %w", err) } return serialized, nil } @@ -180,7 +178,7 @@ func (r *AgentReport) Scan(src any) error { // For serializing to db func (r *AgentReport) Value() (driver.Value, error) { if r == nil { - return nil, nil + return nil, fmt.Errorf("agent report is nil") } serialized, err := json.Marshal(r) @@ -234,7 +232,7 @@ func (c *CloudServiceConfig) Scan(src any) error { // For serializing to db func (c *CloudServiceConfig) Value() (driver.Value, error) { if c == nil { - return nil, nil + return nil, fmt.Errorf("cloud service config is nil") } serialized, err := json.Marshal(c) diff --git a/pkg/types/ruletypes/maintenance.go b/pkg/types/ruletypes/maintenance.go index 911c1c0657..4531baf3ce 100644 --- a/pkg/types/ruletypes/maintenance.go +++ b/pkg/types/ruletypes/maintenance.go @@ -9,7 +9,6 @@ import ( "github.com/SigNoz/signoz/pkg/types" "github.com/SigNoz/signoz/pkg/valuer" "github.com/uptrace/bun" - "go.uber.org/zap" ) var ( @@ -73,11 +72,9 @@ func (m *GettablePlannedMaintenance) ShouldSkip(ruleID string, now time.Time) bo return false } - zap.L().Info("alert found in maintenance", zap.String("alert", ruleID), zap.String("maintenance", m.Name)) // If alert is found, we check if it should be skipped based on the schedule loc, err := time.LoadLocation(m.Schedule.Timezone) if err != nil { - zap.L().Error("Error loading location", zap.String("timezone", m.Schedule.Timezone), zap.Error(err)) return false } @@ -85,13 +82,6 @@ func (m *GettablePlannedMaintenance) ShouldSkip(ruleID string, now time.Time) bo // fixed schedule if !m.Schedule.StartTime.IsZero() && !m.Schedule.EndTime.IsZero() { - zap.L().Info("checking fixed schedule", - zap.String("rule", ruleID), - zap.String("maintenance", m.Name), - zap.Time("currentTime", currentTime), - zap.Time("startTime", m.Schedule.StartTime), - zap.Time("endTime", m.Schedule.EndTime)) - startTime := m.Schedule.StartTime.In(loc) endTime := m.Schedule.EndTime.In(loc) if currentTime.Equal(startTime) || currentTime.Equal(endTime) || @@ -103,19 +93,9 @@ func (m *GettablePlannedMaintenance) ShouldSkip(ruleID string, now time.Time) bo // recurring schedule if m.Schedule.Recurrence != nil { start := m.Schedule.Recurrence.StartTime - duration := time.Duration(m.Schedule.Recurrence.Duration) - - zap.L().Info("checking recurring schedule base info", - zap.String("rule", ruleID), - zap.String("maintenance", m.Name), - zap.Time("startTime", start), - zap.Duration("duration", duration)) // Make sure the recurrence has started if currentTime.Before(start.In(loc)) { - zap.L().Info("current time is before recurrence start time", - zap.String("rule", ruleID), - zap.String("maintenance", m.Name)) return false } @@ -123,9 +103,6 @@ func (m *GettablePlannedMaintenance) ShouldSkip(ruleID string, now time.Time) bo if m.Schedule.Recurrence.EndTime != nil { endTime := *m.Schedule.Recurrence.EndTime if !endTime.IsZero() && currentTime.After(endTime.In(loc)) { - zap.L().Info("current time is after recurrence end time", - zap.String("rule", ruleID), - zap.String("maintenance", m.Name)) return false } } @@ -235,8 +212,6 @@ func (m *GettablePlannedMaintenance) IsActive(now time.Time) bool { func (m *GettablePlannedMaintenance) IsUpcoming() bool { loc, err := time.LoadLocation(m.Schedule.Timezone) if err != nil { - // handle error appropriately, for example log and return false or fallback to UTC - zap.L().Error("Error loading timezone", zap.String("timezone", m.Schedule.Timezone), zap.Error(err)) return false } now := time.Now().In(loc) diff --git a/pkg/types/ssotypes/saml.go b/pkg/types/ssotypes/saml.go index dd318e6edf..c097f5580f 100644 --- a/pkg/types/ssotypes/saml.go +++ b/pkg/types/ssotypes/saml.go @@ -11,7 +11,6 @@ import ( "github.com/SigNoz/signoz/pkg/query-service/constants" saml2 "github.com/russellhaering/gosaml2" dsig "github.com/russellhaering/goxmldsig" - "go.uber.org/zap" ) func LoadCertificateStore(certString string) (dsig.X509CertificateStore, error) { @@ -103,6 +102,6 @@ func PrepareRequest(issuer, acsUrl, audience, entity, idp, certString string) (* IDPCertificateStore: certStore, SPKeyStore: randomKeyStore, } - zap.L().Debug("SAML request", zap.Any("sp", sp)) + return sp, nil } diff --git a/pkg/types/telemetrytypes/telemetrytypestest/metadata_store_stub.go b/pkg/types/telemetrytypes/telemetrytypestest/metadata_store.go similarity index 99% rename from pkg/types/telemetrytypes/telemetrytypestest/metadata_store_stub.go rename to pkg/types/telemetrytypes/telemetrytypestest/metadata_store.go index 0d77ea467d..3be85eeba1 100644 --- a/pkg/types/telemetrytypes/telemetrytypestest/metadata_store_stub.go +++ b/pkg/types/telemetrytypes/telemetrytypestest/metadata_store.go @@ -129,7 +129,7 @@ func (m *MockMetadataStore) GetRelatedValues(ctx context.Context, fieldValueSele // GetAllValues returns all values for a given field func (m *MockMetadataStore) GetAllValues(ctx context.Context, fieldValueSelector *telemetrytypes.FieldValueSelector) (*telemetrytypes.TelemetryFieldValues, error) { if fieldValueSelector == nil { - return nil, nil + return &telemetrytypes.TelemetryFieldValues{}, nil } // Generate a lookup key from the selector