From 1529b79869cd48b3a3437c2ce80b385e17fa7352 Mon Sep 17 00:00:00 2001 From: Gavin Power Date: Sun, 25 Aug 2024 23:59:57 +0100 Subject: [PATCH] Startup Trace and Automatic image switching (#8) * fix: Fixed startup tracing * feat: Added automatic image switchover * Updated go version on PR * Fixed indentation * fix: cleaned up some error handling * fix: linter errors * fix: capitalizations on error messages * fix: Missed one --- .github/workflows/Deploy.yml | 21 ++++++++--- .github/workflows/PR.yml | 2 +- cmd/server/main.go | 45 ++++++++++++++++------- internal/handlers/create_redirect.go | 2 +- internal/handlers/create_redirect_test.go | 6 +-- internal/handlers/redirect_test.go | 4 +- internal/storage/storage_account.go | 12 +++--- pkg/middleware/basic_auth.go | 10 ++--- pkg/observability/otel.go | 17 +++++---- 9 files changed, 74 insertions(+), 45 deletions(-) diff --git a/.github/workflows/Deploy.yml b/.github/workflows/Deploy.yml index 4af7bb5..cbe0ba0 100644 --- a/.github/workflows/Deploy.yml +++ b/.github/workflows/Deploy.yml @@ -7,7 +7,7 @@ on: jobs: - deploy: + build: runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 @@ -28,9 +28,20 @@ jobs: context: . file: ./Dockerfile push: true - tags: ${{ secrets.REGISTRY_URL }}/gavinpower747/shortener:latest + tags: ${{ secrets.REGISTRY_URL }}/gavinpower747/shortener:${{ github.sha }} build-args: | - GIT_COMMIT=${{github.sha}} - GIT_BRANCH=${{github.ref}} + GIT_COMMIT=${{ github.sha }} + GIT_BRANCH=${{ github.ref }} BUILD_DATE=${{ github.event.repository.updated_at }} - + + deploy: + runs-on: ubuntu-latest + needs: build + steps: + - name: 'Deploy new image to app service' + uses: azure/webapps-deploy@v2 + with: + app-name: 'shortener' + slot-name: 'production' + publish_profile: ${{ secrets.AZURE_PUBLISH_PROFILE }} + images: ${{ secrets.REGISTRY_URL }}/gavinpower747/shortener:${{ github.sha }} diff --git a/.github/workflows/PR.yml b/.github/workflows/PR.yml index dfc9f0d..c24fc49 100644 --- a/.github/workflows/PR.yml +++ b/.github/workflows/PR.yml @@ -14,7 +14,7 @@ jobs: - name: 'Set up Go' uses: actions/setup-go@v3 with: - go-version: '1.20' + go-version: '1.23' - name: Verify dependencies run: go mod verify diff --git a/cmd/server/main.go b/cmd/server/main.go index 9052705..4e68792 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -26,31 +26,40 @@ func main() { } } -func run() (err error) { +const ( + ServiceName = "gavs.at" +) +func run() (err error) { ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt) defer stop() - otelShutdown, err := observability.SetupOTelSDK(ctx) + otelShutdown, err := observability.SetupOTelSDK(ctx, ServiceName) if err != nil { - log.Fatalf("Failed to setup otel: %v", err) - return + log.Printf("Failed to setup otel: %v", err) + return err } - tracer := otel.GetTracerProvider().Tracer("gavs.at/shortener") + tracer := otel.GetTracerProvider().Tracer(ServiceName) - _, span := tracer.Start(ctx, "startup") - defer span.End() + ctx, span := tracer.Start(ctx, "ApplicationStartup") defer func() { - err = errors.Join(err, otelShutdown(context.Background())) + if span.IsRecording() && err != nil { + span.RecordError(err) + span.End() + } + + err = errors.Join(err, otelShutdown(ctx)) }() listenAddr := ":80" - storageAccount, err := storage.NewStorageAccount() + storageAccount, err := storage.NewStorageAccount(ctx) if err != nil { - log.Fatal(err) + log.Println(err) + + return err } reqHandlers := handlers.NewHandlers(storageAccount) @@ -68,6 +77,9 @@ func run() (err error) { } srvErr := make(chan error, 1) + + span.End() + go func() { log.Println("Listening on", listenAddr) srvErr <- srv.ListenAndServe() @@ -75,20 +87,25 @@ func run() (err error) { select { case err = <-srvErr: - return + if span.IsRecording() { + span.RecordError(err) + span.End() + } + + return err case <-ctx.Done(): stop() } - err = srv.Shutdown(context.Background()) + err = srv.Shutdown(ctx) - return + return nil } func configureRouter(reqHandlers *handlers.Handlers) *mux.Router { r := mux.NewRouter() - r.Use(otelmux.Middleware("gavs.at")) + r.Use(otelmux.Middleware(ServiceName)) r.Use(middleware.RequestMetrics) apiRouter := r.PathPrefix("/api").Subrouter() diff --git a/internal/handlers/create_redirect.go b/internal/handlers/create_redirect.go index 2085dee..5a7ab3e 100644 --- a/internal/handlers/create_redirect.go +++ b/internal/handlers/create_redirect.go @@ -25,7 +25,7 @@ func (h *Handlers) UpsertRedirect(w http.ResponseWriter, r *http.Request) { Entity: aztables.Entity{PartitionKey: "pk001", RowKey: req.Slug}, } - err := h.storage.UpsertEntity(redirect) + err := h.storage.UpsertEntity(r.Context(), redirect) if err != nil { log.Printf("Error when upserting redirect: %s", err) diff --git a/internal/handlers/create_redirect_test.go b/internal/handlers/create_redirect_test.go index ac2ee0e..5228cb8 100644 --- a/internal/handlers/create_redirect_test.go +++ b/internal/handlers/create_redirect_test.go @@ -23,7 +23,7 @@ func (m *MockStorage) QueryEntity(_ context.Context, _, _ string) ([]byte, error return nil, nil } -func (m *MockStorage) UpsertEntity(entity interface{}) error { +func (m *MockStorage) UpsertEntity(_ context.Context, entity interface{}) error { if m.UpsertEntityFunc == nil { return nil } @@ -34,7 +34,7 @@ func (m *MockStorage) UpsertEntity(entity interface{}) error { func TestHandlers_UpsertRedirect_ValidRequest(t *testing.T) { // Arrange mockStorage := &MockStorage{ - UpsertEntityFunc: func(entity interface{}) error { + UpsertEntityFunc: func(_ interface{}) error { return nil }, } @@ -91,7 +91,7 @@ func TestHandlers_UpsertRedirect_InvalidRequestBody(t *testing.T) { func TestHandlers_UpsertRedirect_StorageError(t *testing.T) { // Arrange mockStorage := &MockStorage{ - UpsertEntityFunc: func(entity interface{}) error { + UpsertEntityFunc: func(_ interface{}) error { return errors.New("test error") }, } diff --git a/internal/handlers/redirect_test.go b/internal/handlers/redirect_test.go index f515fa0..b0eba41 100644 --- a/internal/handlers/redirect_test.go +++ b/internal/handlers/redirect_test.go @@ -24,7 +24,7 @@ type MockStorageAccount struct { mock.Mock } -func (m *MockStorageAccount) QueryEntity(ctx context.Context, partitionKey, rowKey string) ([]byte, error) { +func (m *MockStorageAccount) QueryEntity(_ context.Context, partitionKey, rowKey string) ([]byte, error) { args := m.Called(partitionKey, rowKey) if args.Get(0) == nil { @@ -34,7 +34,7 @@ func (m *MockStorageAccount) QueryEntity(ctx context.Context, partitionKey, rowK return args.Get(0).([]byte), args.Error(1) } -func (m *MockStorageAccount) UpsertEntity(entity interface{}) error { +func (m *MockStorageAccount) UpsertEntity(_ context.Context, entity interface{}) error { args := m.Called(entity) return args.Error(0) diff --git a/internal/storage/storage_account.go b/internal/storage/storage_account.go index 0bcbeea..0026b38 100644 --- a/internal/storage/storage_account.go +++ b/internal/storage/storage_account.go @@ -15,7 +15,7 @@ import ( type Account interface { QueryEntity(ctx context.Context, partitionKey string, rowKey string) ([]byte, error) - UpsertEntity(entity interface{}) error + UpsertEntity(ctx context.Context, entity interface{}) error } type storageAccount struct { @@ -53,7 +53,7 @@ func (sa *storageAccount) QueryEntity(ctx context.Context, partitionKey, rowKey return nil, nil } -func (sa *storageAccount) UpsertEntity(entity interface{}) error { +func (sa *storageAccount) UpsertEntity(ctx context.Context, entity interface{}) error { tableClient := sa.serviceClient.NewClient(TableName) jsonEntity, err := json.Marshal(entity) @@ -62,12 +62,12 @@ func (sa *storageAccount) UpsertEntity(entity interface{}) error { return err } - _, err = tableClient.UpsertEntity(context.Background(), jsonEntity, nil) + _, err = tableClient.UpsertEntity(ctx, jsonEntity, nil) return err } -func NewStorageAccount() (Account, error) { +func NewStorageAccount(ctx context.Context) (Account, error) { connectionString, found := os.LookupEnv("API_AzureStorageConnectionString") if !found { @@ -91,14 +91,14 @@ func NewStorageAccount() (Account, error) { pager := sa.NewListTablesPager(pagerOptions) for pager.More() { - resp, pageErr := pager.NextPage(context.Background()) + resp, pageErr := pager.NextPage(ctx) if pageErr != nil { return nil, pageErr } if len(resp.Tables) == 0 { - _, err = sa.CreateTable(context.Background(), TableName, nil) + _, err = sa.CreateTable(ctx, TableName, nil) if err != nil { return nil, err diff --git a/pkg/middleware/basic_auth.go b/pkg/middleware/basic_auth.go index afd4eca..89e9a1e 100644 --- a/pkg/middleware/basic_auth.go +++ b/pkg/middleware/basic_auth.go @@ -34,7 +34,7 @@ func BasicAuth(next http.Handler) http.Handler { if authHeader == "" { web.NotAuthorized(w, "Missing Authorization Header") - span.RecordError(fmt.Errorf("Missing Authorization Header")) + span.RecordError(fmt.Errorf("missing authorization header")) return } @@ -42,8 +42,8 @@ func BasicAuth(next http.Handler) http.Handler { headerSections := strings.Split(authHeader, " ") if !strings.HasPrefix(authHeader, "Basic ") { - web.NotAuthorized(w, fmt.Sprintf("Invalid Authorization Header, %s authentication scheme is not supported", headerSections[0])) - span.RecordError(fmt.Errorf("Invalid Authorization Header, %s authentication scheme is not supported", headerSections[0])) + web.NotAuthorized(w, fmt.Sprintf("invalid authorization header, %s authentication scheme is not supported", headerSections[0])) + span.RecordError(fmt.Errorf("invalid authorization header, %s authentication scheme is not supported", headerSections[0])) return } @@ -52,7 +52,7 @@ func BasicAuth(next http.Handler) http.Handler { if err != nil { web.NotAuthorized(w, "Invalid Authorization Header") - span.RecordError(fmt.Errorf("Invalid Authorization Header")) + span.RecordError(fmt.Errorf("invalid authorization header")) return } @@ -68,7 +68,7 @@ func BasicAuth(next http.Handler) http.Handler { if username != expectedUsername || passwordHash != expectedPasswordHash { web.NotAuthorized(w, "Invalid Credentials") - span.RecordError(fmt.Errorf("Invalid Credentials")) + span.RecordError(fmt.Errorf("invalid credentials")) return } diff --git a/pkg/observability/otel.go b/pkg/observability/otel.go index 3068b50..3ec383c 100644 --- a/pkg/observability/otel.go +++ b/pkg/observability/otel.go @@ -14,16 +14,16 @@ import ( semconv "go.opentelemetry.io/otel/semconv/v1.4.0" ) -func SetupOTelSDK(ctx context.Context) (shutdown func(context.Context) error, err error) { +func SetupOTelSDK(ctx context.Context, serviceName string) (shutdown func(context.Context) error, err error) { var shutdownFuncs []func(context.Context) error shutdown = func(ctx context.Context) error { - var err error for _, fn := range shutdownFuncs { if shutErr := fn(ctx); shutErr != nil { err = errors.Join(err, shutErr) } } + return err } @@ -33,9 +33,9 @@ func SetupOTelSDK(ctx context.Context) (shutdown func(context.Context) error, er } } - resource := resource.NewWithAttributes( + otelService := resource.NewWithAttributes( semconv.SchemaURL, - semconv.ServiceNameKey.String("gavs.at"), + semconv.ServiceNameKey.String(serviceName), semconv.ServiceVersionKey.String(os.Getenv("GIT_COMMIT")), attribute.String("build.commit", os.Getenv("GIT_COMMIT")), attribute.String("build.branch", os.Getenv("GIT_BRANCH")), @@ -45,10 +45,10 @@ func SetupOTelSDK(ctx context.Context) (shutdown func(context.Context) error, er otel.SetTextMapPropagator(newPropagator()) - tracerProvider, err := newTraceProvider(ctx, resource) + tracerProvider, err := newTraceProvider(ctx, otelService) if err != nil { handleErr(err) - return + return shutdown, err } shutdownFuncs = append(shutdownFuncs, tracerProvider.Shutdown) @@ -64,7 +64,7 @@ func newPropagator() propagation.TextMapPropagator { ) } -func newTraceProvider(ctx context.Context, resource *resource.Resource) (*trace.TracerProvider, error) { +func newTraceProvider(ctx context.Context, service *resource.Resource) (*trace.TracerProvider, error) { exporter, err := otlptracehttp.New(ctx) if err != nil { @@ -73,7 +73,8 @@ func newTraceProvider(ctx context.Context, resource *resource.Resource) (*trace. tp := trace.NewTracerProvider( trace.WithBatcher(exporter), - trace.WithResource(resource), + trace.WithResource(service), ) + return tp, nil }