From 9252fff4d6cff8d2a56a48f1a70ff6219e389c79 Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Wed, 15 Mar 2023 08:56:28 +0300 Subject: [PATCH] [MM-49993] Return error if there is no telemetry ID (#22438) * return error if there is no telemetry ID * fix tests and apply review comments --- app/server.go | 6 +++- services/telemetry/telemetry.go | 42 +++++++++++++++++++--------- services/telemetry/telemetry_test.go | 18 ++++++++---- 3 files changed, 46 insertions(+), 20 deletions(-) diff --git a/app/server.go b/app/server.go index 490b3ce870..5d0f774eb8 100644 --- a/app/server.go +++ b/app/server.go @@ -360,7 +360,11 @@ func NewServer(options ...Option) (*Server, error) { }) s.htmlTemplateWatcher = htmlTemplateWatcher - s.telemetryService = telemetry.New(New(ServerConnector(s.Channels())), s.Store(), s.platform.SearchEngine, s.Log(), *s.Config().LogSettings.VerboseDiagnostics) + s.telemetryService, err = telemetry.New(New(ServerConnector(s.Channels())), s.Store(), s.platform.SearchEngine, s.Log(), *s.Config().LogSettings.VerboseDiagnostics) + if err != nil { + return nil, errors.Wrapf(err, "unable to initialize telemetry service") + } + s.platform.SetTelemetryId(s.TelemetryId()) // TODO: move this into platform once telemetry service moved to platform. emailService, err := email.NewService(email.ServiceConfig{ diff --git a/services/telemetry/telemetry.go b/services/telemetry/telemetry.go index 5554da8d2d..b34cea3eba 100644 --- a/services/telemetry/telemetry.go +++ b/services/telemetry/telemetry.go @@ -5,6 +5,7 @@ package telemetry import ( "context" + "fmt" "os" "path/filepath" "runtime" @@ -25,8 +26,10 @@ import ( ) const ( - DayMilliseconds = 24 * 60 * 60 * 1000 - MonthMilliseconds = 31 * DayMilliseconds + DayMilliseconds = 24 * 60 * 60 * 1000 + MonthMilliseconds = 31 * DayMilliseconds + DBAccessAttempts = 3 + DBAccessTimeoutSecs = 10 RudderKey = "placeholder_rudder_key" RudderDataplaneURL = "placeholder_rudder_dataplane_url" @@ -111,7 +114,7 @@ type RudderConfig struct { DataplaneURL string } -func New(srv ServerIface, dbStore store.Store, searchEngine *searchengine.Broker, log *mlog.Logger, verbose bool) *TelemetryService { +func New(srv ServerIface, dbStore store.Store, searchEngine *searchengine.Broker, log *mlog.Logger, verbose bool) (*TelemetryService, error) { service := &TelemetryService{ srv: srv, dbStore: dbStore, @@ -119,24 +122,37 @@ func New(srv ServerIface, dbStore store.Store, searchEngine *searchengine.Broker log: log, verbose: verbose, } - service.ensureTelemetryID() - return service + + if err := service.ensureTelemetryID(); err != nil { + return nil, fmt.Errorf("unable to ensure telemetry ID: %w", err) + } + + return service, nil } -func (ts *TelemetryService) ensureTelemetryID() { +func (ts *TelemetryService) ensureTelemetryID() error { if ts.TelemetryID != "" { - return + return nil } id := model.NewId() - systemID := &model.System{Name: model.SystemTelemetryId, Value: id} - systemID, err := ts.dbStore.System().InsertIfExists(systemID) - if err != nil { - ts.log.Error("unable to get the telemetry ID", mlog.Err(err)) - return + var err error + + for i := 0; i < DBAccessAttempts; i++ { + ts.log.Info("Ensuring the telemetry ID", mlog.String("id", id)) + systemID := &model.System{Name: model.SystemTelemetryId, Value: id} + systemID, err = ts.dbStore.System().InsertIfExists(systemID) + if err != nil { + ts.log.Info("Unable to get/set the telemetry ID", mlog.Err(err)) + time.Sleep(DBAccessTimeoutSecs * time.Second) + continue + } + + ts.TelemetryID = systemID.Value + return nil } - ts.TelemetryID = systemID.Value + return fmt.Errorf("unable to get the telemetry ID: %w", err) } func (ts *TelemetryService) getRudderConfig() RudderConfig { diff --git a/services/telemetry/telemetry_test.go b/services/telemetry/telemetry_test.go index 650794eb03..bb8cf8876b 100644 --- a/services/telemetry/telemetry_test.go +++ b/services/telemetry/telemetry_test.go @@ -119,7 +119,9 @@ func makeTelemetryServiceAndReceiver(t *testing.T, cloudLicense bool) (*Telemetr pchan <- p })) - service := New(serverIfaceMock, storeMock, searchengine.NewBroker(cfg), testLogger, false) + service, err := New(serverIfaceMock, storeMock, searchengine.NewBroker(cfg), testLogger, false) + require.NoError(t, err) + service.TelemetryID = testTelemetryID service.rudderClient = nil service.initRudder(receiver.URL, RudderKey) @@ -297,7 +299,9 @@ func TestEnsureTelemetryID(t *testing.T) { testLogger, _ := mlog.NewLogger() - telemetryService := New(serverIfaceMock, storeMock, searchengine.NewBroker(cfg), testLogger, false) + telemetryService, err := New(serverIfaceMock, storeMock, searchengine.NewBroker(cfg), testLogger, false) + require.NoError(t, err) + assert.Equal(t, "test", telemetryService.TelemetryID) telemetryService.ensureTelemetryID() @@ -330,7 +334,9 @@ func TestEnsureTelemetryID(t *testing.T) { testLogger, _ := mlog.NewLogger() - telemetryService := New(serverIfaceMock, storeMock, searchengine.NewBroker(cfg), testLogger, false) + telemetryService, err := New(serverIfaceMock, storeMock, searchengine.NewBroker(cfg), testLogger, false) + require.NoError(t, err) + assert.Equal(t, generatedID, telemetryService.TelemetryID) }) @@ -340,7 +346,7 @@ func TestEnsureTelemetryID(t *testing.T) { systemStore := storeMocks.SystemStore{} insertError := errors.New("insert error") - systemStore.On("InsertIfExists", mock.AnythingOfType("*model.System")).Return(nil, insertError).Once() + systemStore.On("InsertIfExists", mock.AnythingOfType("*model.System")).Return(nil, insertError).Times(DBAccessAttempts) storeMock.On("System").Return(&systemStore) @@ -350,8 +356,8 @@ func TestEnsureTelemetryID(t *testing.T) { testLogger, _ := mlog.NewLogger() - telemetryService := New(serverIfaceMock, storeMock, searchengine.NewBroker(cfg), testLogger, false) - assert.Equal(t, "", telemetryService.TelemetryID) + _, err := New(serverIfaceMock, storeMock, searchengine.NewBroker(cfg), testLogger, false) + require.Error(t, err) }) }