Skip to content

Improve error logging #629

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Improve error logging #629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions packages/api/internal/auth/middleware.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"errors"
"fmt"
"log"
"net/http"
"os"
"strings"
Expand Down Expand Up @@ -82,7 +81,7 @@ func (a *commonAuthenticator[T]) Authenticate(ctx context.Context, input *openap
// Now, we need to get the API key from the request
headerKey, err := a.getHeaderKeysFromRequest(input.RequestValidationInput.Request)
if err != nil {
telemetry.ReportCriticalError(ctx, fmt.Errorf("%v %w", a.errorMessage, err))
telemetry.ReportCriticalError(ctx, a.errorMessage, err)

return fmt.Errorf("%v %w", a.errorMessage, err)
}
Expand All @@ -92,8 +91,7 @@ func (a *commonAuthenticator[T]) Authenticate(ctx context.Context, input *openap
// If the API key is valid, we will get a result back
result, validationError := a.validationFunction(ctx, headerKey)
if validationError != nil {
log.Printf("validation error %v", validationError.Err)
telemetry.ReportError(ctx, fmt.Errorf("%s %w", a.errorMessage, validationError.Err))
telemetry.ReportError(ctx, a.errorMessage, validationError.Err)

return fmt.Errorf(a.errorMessage)
}
Expand Down
2 changes: 1 addition & 1 deletion packages/api/internal/cache/instance/instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func (c *InstanceCache) Set(key string, value *InstanceInfo) {
go func() {
err := c.insertInstance(value)
if err != nil {
fmt.Printf("error inserting instance: %v", err)
zap.L().Error("error inserting instance", zap.Error(err))
}
}()
}
Expand Down
17 changes: 7 additions & 10 deletions packages/api/internal/handlers/accesstoken.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ func (a *APIStore) PostAccessTokens(c *gin.Context) {
if err != nil {
a.sendAPIStoreError(c, http.StatusBadRequest, fmt.Sprintf("Error when parsing request: %s", err))

errMsg := fmt.Errorf("error when parsing request: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when parsing request", err)

return
}
Expand All @@ -33,8 +32,7 @@ func (a *APIStore) PostAccessTokens(c *gin.Context) {
if err != nil {
a.sendAPIStoreError(c, http.StatusInternalServerError, fmt.Sprintf("Error when generating access token: %s", err))

errMsg := fmt.Errorf("error when generating access token: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when generating access token", err)

return
}
Expand All @@ -52,8 +50,7 @@ func (a *APIStore) PostAccessTokens(c *gin.Context) {
if err != nil {
a.sendAPIStoreError(c, http.StatusInternalServerError, fmt.Sprintf("Error when creating access token: %s", err))

errMsg := fmt.Errorf("error when creating access token: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when creating access token", err)

return
}
Expand All @@ -76,8 +73,8 @@ func (a *APIStore) DeleteAccessTokensAccessTokenID(c *gin.Context, accessTokenID
if err != nil {
a.sendAPIStoreError(c, http.StatusBadRequest, fmt.Sprintf("Error when parsing access token ID: %s", err))

errMsg := fmt.Errorf("error when parsing access token ID: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when parsing access token ID", err)

return
}

Expand All @@ -90,8 +87,8 @@ func (a *APIStore) DeleteAccessTokensAccessTokenID(c *gin.Context, accessTokenID
} else if err != nil {
a.sendAPIStoreError(c, http.StatusInternalServerError, fmt.Sprintf("Error when deleting access token: %s", err))

errMsg := fmt.Errorf("error when deleting access token: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when deleting access token", err)

return
}

Expand Down
3 changes: 1 addition & 2 deletions packages/api/internal/handlers/admin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ func (a *APIStore) PostNodesNodeID(c *gin.Context, nodeId api.NodeID) {
if err != nil {
a.sendAPIStoreError(c, http.StatusBadRequest, fmt.Sprintf("Error when parsing request: %s", err))

errMsg := fmt.Errorf("error when parsing request: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when parsing request", err)

return
}
Expand Down
28 changes: 10 additions & 18 deletions packages/api/internal/handlers/apikey.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@

import (
"fmt"
"log"
"net/http"
"strings"
"time"

"github.com/gin-gonic/gin"
"github.com/google/uuid"
"go.uber.org/zap"

"github.com/e2b-dev/infra/packages/api/internal/api"
"github.com/e2b-dev/infra/packages/api/internal/team"
Expand All @@ -26,17 +26,15 @@
if err != nil {
a.sendAPIStoreError(c, http.StatusBadRequest, fmt.Sprintf("Error when parsing request: %s", err))

errMsg := fmt.Errorf("error when parsing request: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when parsing request", err)
return
}

apiKeyIDParsed, err := uuid.Parse(apiKeyID)
if err != nil {
a.sendAPIStoreError(c, http.StatusBadRequest, fmt.Sprintf("Error when parsing API key ID: %s", err))

errMsg := fmt.Errorf("error when parsing API key ID: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when parsing API key ID", err)
return
}

Expand All @@ -47,8 +45,7 @@
} else if err != nil {
a.sendAPIStoreError(c, http.StatusInternalServerError, fmt.Sprintf("Error when updating team API key name: %s", err))

errMsg := fmt.Errorf("error when updating team API key name: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when updating team API key name", err)
return
}

Expand All @@ -66,7 +63,7 @@
WithCreator().
All(ctx)
if err != nil {
log.Println("Error when getting team API keys: ", err)
zap.L().Warn("error when getting team API keys", zap.Error(err))

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by an access to TeamAPIKey
Loading
flows to a logging call.

Copilot Autofix

AI 7 days ago

To fix the issue, we should ensure that sensitive information is not logged in plaintext. Instead of logging the raw error object, we can log a generic error message that does not include sensitive details. If additional context is needed for debugging, we can log a sanitized or obfuscated version of the error message. This approach ensures that sensitive data is not exposed while still providing useful information for troubleshooting.

Suggested changeset 1
packages/api/internal/handlers/apikey.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/api/internal/handlers/apikey.go b/packages/api/internal/handlers/apikey.go
--- a/packages/api/internal/handlers/apikey.go
+++ b/packages/api/internal/handlers/apikey.go
@@ -65,3 +65,3 @@
 	if err != nil {
-		zap.L().Warn("error when getting team API keys", zap.Error(err))
+		zap.L().Warn("error when getting team API keys", zap.String("error", "an error occurred while querying team API keys"))
 		c.String(http.StatusInternalServerError, "Error when getting team API keys")
EOF
@@ -65,3 +65,3 @@
if err != nil {
zap.L().Warn("error when getting team API keys", zap.Error(err))
zap.L().Warn("error when getting team API keys", zap.String("error", "an error occurred while querying team API keys"))
c.String(http.StatusInternalServerError, "Error when getting team API keys")
Copilot is powered by AI and may make mistakes. Always verify output.
c.String(http.StatusInternalServerError, "Error when getting team API keys")

return
Expand Down Expand Up @@ -110,8 +107,7 @@
if err != nil {
a.sendAPIStoreError(c, http.StatusBadRequest, fmt.Sprintf("Error when parsing API key ID: %s", err))

errMsg := fmt.Errorf("error when parsing API key ID: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when parsing API key ID", err)
return
}

Expand All @@ -122,8 +118,7 @@
} else if err != nil {
a.sendAPIStoreError(c, http.StatusInternalServerError, fmt.Sprintf("Error when deleting API key: %s", err))

errMsg := fmt.Errorf("error when deleting API key: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when deleting API key", err)
return
}

Expand All @@ -140,8 +135,7 @@
if err != nil {
a.sendAPIStoreError(c, http.StatusBadRequest, fmt.Sprintf("Error when parsing request: %s", err))

errMsg := fmt.Errorf("error when parsing request: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when parsing request", err)

return
}
Expand All @@ -150,8 +144,7 @@
if err != nil {
a.sendAPIStoreError(c, http.StatusInternalServerError, fmt.Sprintf("Error when creating team API key: %s", err))

errMsg := fmt.Errorf("error when creating team API key: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when creating team API key", err)

return
}
Expand All @@ -160,8 +153,7 @@
if err != nil {
a.sendAPIStoreError(c, http.StatusInternalServerError, fmt.Sprintf("Error when getting user: %s", err))

errMsg := fmt.Errorf("error when getting user: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when getting user", err)

return
}
Expand Down
4 changes: 1 addition & 3 deletions packages/api/internal/handlers/sandbox.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package handlers

import (
"context"
"fmt"
"net/http"
"time"

Expand Down Expand Up @@ -53,8 +52,7 @@ func (a *APIStore) startSandbox(
envdAccessToken,
)
if instanceErr != nil {
errMsg := fmt.Errorf("error when creating instance: %w", instanceErr.Err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when creating instance", instanceErr.Err)
return nil, instanceErr
}

Expand Down
8 changes: 3 additions & 5 deletions packages/api/internal/handlers/sandbox_create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ func (a *APIStore) PostSandboxes(c *gin.Context) {
if err != nil {
a.sendAPIStoreError(c, http.StatusBadRequest, fmt.Sprintf("Error when parsing request: %s", err))

errMsg := fmt.Errorf("error when parsing request: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when parsing request", err)

return
}
Expand All @@ -67,8 +66,7 @@ func (a *APIStore) PostSandboxes(c *gin.Context) {
if err != nil {
a.sendAPIStoreError(c, http.StatusBadRequest, fmt.Sprintf("Invalid environment ID: %s", err))

errMsg := fmt.Errorf("error when cleaning env ID: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when cleaning env ID", err)

return
}
Expand All @@ -81,7 +79,7 @@ func (a *APIStore) PostSandboxes(c *gin.Context) {
// Check if team has access to the environment
env, build, checkErr := a.templateCache.Get(ctx, cleanedAliasOrEnvID, teamInfo.Team.ID, true)
if checkErr != nil {
telemetry.ReportCriticalError(ctx, checkErr.Err)
telemetry.ReportCriticalError(ctx, "error when getting template", checkErr.Err)
a.sendAPIStoreError(c, checkErr.Code, checkErr.ClientMsg)
return
}
Expand Down
3 changes: 1 addition & 2 deletions packages/api/internal/handlers/sandbox_kill.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ func (a *APIStore) DeleteSandboxesSandboxID(
sbx, err := a.orchestrator.GetSandbox(sandboxID)
if err == nil {
if *sbx.TeamID != teamID {
errMsg := fmt.Errorf("sandbox '%s' does not belong to team '%s'", sandboxID, teamID.String())
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "sandbox does not belong to team", fmt.Errorf("sandbox '%s' does not belong to team '%s'", sandboxID, teamID.String()))

a.sendAPIStoreError(c, http.StatusUnauthorized, fmt.Sprintf("Error deleting sandbox - sandbox '%s' does not belong to your team '%s'", sandboxID, teamID.String()))

Expand Down
6 changes: 2 additions & 4 deletions packages/api/internal/handlers/sandbox_logs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ func (a *APIStore) GetSandboxesSandboxIDLogs(

res, err := a.lokiClient.QueryRange(query, int(*params.Limit), start, end, logproto.FORWARD, time.Duration(0), time.Duration(0), true)
if err != nil {
errMsg := fmt.Errorf("error when returning logs for sandbox: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when returning logs for sandbox", err)
a.sendAPIStoreError(c, http.StatusNotFound, fmt.Sprintf("Error returning logs for sandbox '%s'", sandboxID))

return
Expand Down Expand Up @@ -87,8 +86,7 @@ func (a *APIStore) GetSandboxesSandboxIDLogs(
})

default:
errMsg := fmt.Errorf("unexpected value type %T", res.Data.Result.Type())
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "unexpected value type", fmt.Errorf("unexpected value type %T", res.Data.Result.Type()))
a.sendAPIStoreError(c, http.StatusInternalServerError, fmt.Sprintf("Error returning logs for sandbox '%s", sandboxID))

return
Expand Down
10 changes: 2 additions & 8 deletions packages/api/internal/handlers/sandbox_metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"github.com/grafana/loki/pkg/loghttp"
"github.com/grafana/loki/pkg/logproto"
"go.opentelemetry.io/otel/attribute"
"go.uber.org/zap"

"github.com/e2b-dev/infra/packages/api/internal/api"
"github.com/e2b-dev/infra/packages/api/internal/auth"
Expand Down Expand Up @@ -70,8 +69,7 @@ func (a *APIStore) LegacyGetSandboxIDMetrics(

err := json.Unmarshal([]byte(entry.Line), &metric)
if err != nil {
zap.L().Error("Failed to unmarshal metric", zap.String("sandbox_id", sandboxID), zap.Error(err))
telemetry.ReportCriticalError(ctx, fmt.Errorf("failed to unmarshal metric: %w", err))
telemetry.ReportCriticalError(ctx, "failed to unmarshal metric", err, attribute.String("sandbox_id", sandboxID))

continue
}
Expand Down Expand Up @@ -174,11 +172,7 @@ func (a *APIStore) GetSandboxesSandboxIDMetrics(
metrics, err := a.readMetricsBasedOnConfig(ctx, sandboxID, teamID, a)

if err != nil {
zap.L().Error("Error returning metrics for sandbox",
zap.Error(err),
zap.String("sandboxID", sandboxID),
)
telemetry.ReportCriticalError(ctx, err)
telemetry.ReportCriticalError(ctx, "error returning metrics for sandbox", err, attribute.String("sandboxID", sandboxID))
a.sendAPIStoreError(c, http.StatusInternalServerError, fmt.Sprintf("Error returning metrics for sandbox '%s'", sandboxID))

return
Expand Down
3 changes: 1 addition & 2 deletions packages/api/internal/handlers/sandbox_pause.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ func (a *APIStore) PostSandboxesSandboxIDPause(c *gin.Context, sandboxID api.San
}

if *sbx.TeamID != teamID {
errMsg := fmt.Errorf("sandbox '%s' does not belong to team '%s'", sandboxID, teamID.String())
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "sandbox does not belong to team", fmt.Errorf("sandbox '%s' does not belong to team '%s'", sandboxID, teamID.String()))

a.sendAPIStoreError(c, http.StatusUnauthorized, fmt.Sprintf("Error pausing sandbox - sandbox '%s' does not belong to your team '%s'", sandboxID, teamID.String()))

Expand Down
5 changes: 2 additions & 3 deletions packages/api/internal/handlers/sandbox_refresh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ func (a *APIStore) PostSandboxesSandboxIDRefreshes(
if err != nil {
a.sendAPIStoreError(c, http.StatusBadRequest, fmt.Sprintf("Error when parsing request: %s", err))

errMsg := fmt.Errorf("error when parsing request: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when parsing request", err)

return
}
Expand All @@ -44,7 +43,7 @@ func (a *APIStore) PostSandboxesSandboxIDRefreshes(

apiErr := a.orchestrator.KeepAliveFor(ctx, sandboxID, duration, false)
if apiErr != nil {
telemetry.ReportCriticalError(ctx, apiErr.Err)
telemetry.ReportCriticalError(ctx, "error when refreshing sandbox", apiErr.Err)
a.sendAPIStoreError(c, apiErr.Code, apiErr.ClientMsg)

return
Expand Down
3 changes: 1 addition & 2 deletions packages/api/internal/handlers/sandbox_resume.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ func (a *APIStore) PostSandboxesSandboxIDResume(c *gin.Context, sandboxID api.Sa
if err != nil {
a.sendAPIStoreError(c, http.StatusBadRequest, fmt.Sprintf("Error when parsing request: %s", err))

errMsg := fmt.Errorf("error when parsing request: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when parsing request", err)

return
}
Expand Down
5 changes: 2 additions & 3 deletions packages/api/internal/handlers/sandbox_timeout.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ func (a *APIStore) PostSandboxesSandboxIDTimeout(
if err != nil {
a.sendAPIStoreError(c, http.StatusBadRequest, fmt.Sprintf("Error when parsing request: %s", err))

errMsg := fmt.Errorf("error when parsing request: %w", err)
telemetry.ReportCriticalError(ctx, errMsg)
telemetry.ReportCriticalError(ctx, "error when parsing request", err)

return
}
Expand All @@ -39,7 +38,7 @@ func (a *APIStore) PostSandboxesSandboxIDTimeout(

apiErr := a.orchestrator.KeepAliveFor(ctx, sandboxID, duration, true)
if apiErr != nil {
telemetry.ReportCriticalError(ctx, apiErr.Err)
telemetry.ReportCriticalError(ctx, "error when setting timeout", apiErr.Err)
a.sendAPIStoreError(c, apiErr.Code, apiErr.ClientMsg)

return
Expand Down
3 changes: 1 addition & 2 deletions packages/api/internal/handlers/sandboxes_list_metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,7 @@ func (a *APIStore) GetSandboxesMetrics(c *gin.Context, params api.GetSandboxesMe

sandboxesWithMetrics, err := a.getSandboxesMetrics(ctx, team.ID, sandboxes)
if err != nil {
zap.L().Error("Error fetching metrics for sandboxes", zap.Error(err))
telemetry.ReportCriticalError(ctx, err)
telemetry.ReportCriticalError(ctx, "error fetching metrics for sandboxes", err)
a.sendAPIStoreError(c, http.StatusInternalServerError, fmt.Sprintf("Error returning metrics for sandboxes for team '%s'", team.ID))

return
Expand Down
Loading
Loading