Skip to content
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.

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

Sentry Performance Tracing #275

Merged
merged 5 commits into from
Feb 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 5 additions & 1 deletion cmd/poseidon/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"github.com/getsentry/sentry-go"
sentryhttp "github.com/getsentry/sentry-go/http"
"github.com/openHPI/poseidon/internal/api"
"github.com/openHPI/poseidon/internal/config"
"github.com/openHPI/poseidon/internal/environment"
Expand Down Expand Up @@ -102,6 +103,9 @@ func initServer() *http.Server {
runnerManager, environmentManager = createManagerHandler(createAWSManager, config.Config.AWS.Enabled,
runnerManager, environmentManager)

handler := api.NewRouter(runnerManager, environmentManager)
sentryHandler := sentryhttp.New(sentryhttp.Options{}).Handle(handler)

return &http.Server{
Addr: config.Config.Server.URL().Host,
// A WriteTimeout would prohibit long-running requests such as creating an execution environment.
Expand All @@ -110,7 +114,7 @@ func initServer() *http.Server {
ReadHeaderTimeout: time.Second * 15,
ReadTimeout: time.Second * 15,
IdleTimeout: time.Second * 60,
Handler: api.NewRouter(runnerManager, environmentManager),
Handler: sentryHandler,
}
}

Expand Down
6 changes: 6 additions & 0 deletions configuration.example.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ sentry:
# This release information is used by Poseidon to provide the version route.
# Normally it is set by the deployment process.
# release: this is replaced in the deployment process
# In debug mode, the debug information is printed to stdout to help you understand what sentry is doing.
# debug: true
# Enable performance tracing.
# enabletracing: true
# The sample rate for sampling traces in the range [0.0, 1.0].
# tracessamplerate: 1.0

# Configuration of the influxdb monitoring
influxdb:
Expand Down
2 changes: 1 addition & 1 deletion deploy/codeocean-terraform
Submodule codeocean-terraform updated from 0b456c to 38ffc3
7 changes: 6 additions & 1 deletion internal/api/environments.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package api

import (
"context"
"encoding/json"
"errors"
"fmt"
"github.com/gorilla/mux"
"github.com/openHPI/poseidon/internal/environment"
"github.com/openHPI/poseidon/internal/runner"
"github.com/openHPI/poseidon/pkg/dto"
"github.com/openHPI/poseidon/pkg/logging"
"net/http"
"strconv"
)
Expand Down Expand Up @@ -118,7 +120,10 @@ func (e *EnvironmentController) createOrUpdate(writer http.ResponseWriter, reque
return
}

created, err := e.manager.CreateOrUpdate(environmentID, *req)
var created bool
logging.StartSpan("api.env.update", "Create Environment", request.Context(), func(ctx context.Context) {
created, err = e.manager.CreateOrUpdate(environmentID, *req, ctx)
})
if err != nil {
writeInternalServerError(writer, err, dto.ErrorUnknown)
}
Expand Down
6 changes: 3 additions & 3 deletions internal/api/environments_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ func (s *CreateOrUpdateEnvironmentTestSuite) TestReturnsBadRequestWhenBadBody()
func (s *CreateOrUpdateEnvironmentTestSuite) TestReturnsInternalServerErrorWhenManagerReturnsError() {
testError := tests.ErrDefault
s.manager.
On("CreateOrUpdate", s.id, mock.AnythingOfType("dto.ExecutionEnvironmentRequest")).
On("CreateOrUpdate", s.id, mock.AnythingOfType("dto.ExecutionEnvironmentRequest"), mock.Anything).
Return(false, testError)

recorder := s.recordRequest()
Expand All @@ -256,7 +256,7 @@ func (s *CreateOrUpdateEnvironmentTestSuite) TestReturnsInternalServerErrorWhenM

func (s *CreateOrUpdateEnvironmentTestSuite) TestReturnsCreatedIfNewEnvironment() {
s.manager.
On("CreateOrUpdate", s.id, mock.AnythingOfType("dto.ExecutionEnvironmentRequest")).
On("CreateOrUpdate", s.id, mock.AnythingOfType("dto.ExecutionEnvironmentRequest"), mock.Anything).
Return(true, nil)

recorder := s.recordRequest()
Expand All @@ -265,7 +265,7 @@ func (s *CreateOrUpdateEnvironmentTestSuite) TestReturnsCreatedIfNewEnvironment(

func (s *CreateOrUpdateEnvironmentTestSuite) TestReturnsNoContentIfNotNewEnvironment() {
s.manager.
On("CreateOrUpdate", s.id, mock.AnythingOfType("dto.ExecutionEnvironmentRequest")).
On("CreateOrUpdate", s.id, mock.AnythingOfType("dto.ExecutionEnvironmentRequest"), mock.Anything).
Return(false, nil)

recorder := s.recordRequest()
Expand Down
32 changes: 26 additions & 6 deletions internal/api/runners.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package api

import (
"context"
"errors"
"fmt"
"github.com/google/uuid"
Expand Down Expand Up @@ -65,7 +66,11 @@ func (r *RunnerController) provide(writer http.ResponseWriter, request *http.Req
}
environmentID := dto.EnvironmentID(runnerRequest.ExecutionEnvironmentID)

nextRunner, err := r.manager.Claim(environmentID, runnerRequest.InactivityTimeout)
var nextRunner runner.Runner
var err error
logging.StartSpan("api.runner.claim", "Claim Runner", request.Context(), func(_ context.Context) {
nextRunner, err = r.manager.Claim(environmentID, runnerRequest.InactivityTimeout)
})
if err != nil {
switch {
case errors.Is(err, runner.ErrUnknownExecutionEnvironment):
Expand Down Expand Up @@ -103,7 +108,9 @@ func (r *RunnerController) listFileSystem(writer http.ResponseWriter, request *h
}

writer.Header().Set("Content-Type", "application/json")
err = targetRunner.ListFileSystem(path, recursive, writer, privilegedExecution, request.Context())
logging.StartSpan("api.fs.list", "List File System", request.Context(), func(ctx context.Context) {
err = targetRunner.ListFileSystem(path, recursive, writer, privilegedExecution, ctx)
})
if errors.Is(err, runner.ErrFileNotFound) {
writeClientError(writer, err, http.StatusFailedDependency)
return
Expand All @@ -125,7 +132,12 @@ func (r *RunnerController) updateFileSystem(writer http.ResponseWriter, request

targetRunner, _ := runner.FromContext(request.Context())
monitoring.AddRunnerMonitoringData(request, targetRunner.ID(), targetRunner.Environment())
if err := targetRunner.UpdateFileSystem(fileCopyRequest); err != nil {

var err error
logging.StartSpan("api.fs.update", "Update File System", request.Context(), func(ctx context.Context) {
err = targetRunner.UpdateFileSystem(fileCopyRequest, ctx)
})
if err != nil {
log.WithError(err).Error("Could not perform the requested updateFileSystem.")
writeInternalServerError(writer, err, dto.ErrorUnknown)
return
Expand All @@ -144,7 +156,9 @@ func (r *RunnerController) fileContent(writer http.ResponseWriter, request *http
}

writer.Header().Set("Content-Disposition", "attachment; filename=\""+path+"\"")
err = targetRunner.GetFileContent(path, writer, privilegedExecution, request.Context())
logging.StartSpan("api.fs.read", "File Content", request.Context(), func(ctx context.Context) {
err = targetRunner.GetFileContent(path, writer, privilegedExecution, ctx)
})
if errors.Is(err, runner.ErrFileNotFound) {
writeClientError(writer, err, http.StatusFailedDependency)
return
Expand Down Expand Up @@ -191,7 +205,10 @@ func (r *RunnerController) execute(writer http.ResponseWriter, request *http.Req
return
}
id := newUUID.String()
targetRunner.StoreExecution(id, executionRequest)

logging.StartSpan("api.runner.exec", "Store Execution", request.Context(), func(ctx context.Context) {
targetRunner.StoreExecution(id, executionRequest)
})
webSocketURL := url.URL{
Scheme: scheme,
Host: request.Host,
Expand Down Expand Up @@ -230,7 +247,10 @@ func (r *RunnerController) delete(writer http.ResponseWriter, request *http.Requ
targetRunner, _ := runner.FromContext(request.Context())
monitoring.AddRunnerMonitoringData(request, targetRunner.ID(), targetRunner.Environment())

err := r.manager.Return(targetRunner)
var err error
logging.StartSpan("api.runner.delete", "Return Runner", request.Context(), func(ctx context.Context) {
err = r.manager.Return(targetRunner)
})
if err != nil {
writeInternalServerError(writer, err, dto.ErrorNomadInternalServerError)
return
Expand Down
8 changes: 5 additions & 3 deletions internal/api/runners_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,8 @@ func (s *UpdateFileSystemRouteTestSuite) SetupTest() {
}

func (s *UpdateFileSystemRouteTestSuite) TestUpdateFileSystemReturnsNoContentOnValidRequest() {
s.runnerMock.On("UpdateFileSystem", mock.AnythingOfType("*dto.UpdateFileSystemRequest")).Return(nil)
s.runnerMock.On("UpdateFileSystem", mock.AnythingOfType("*dto.UpdateFileSystemRequest"), mock.Anything).
Return(nil)

copyRequest := dto.UpdateFileSystemRequest{}
body, err := json.Marshal(copyRequest)
Expand All @@ -287,7 +288,8 @@ func (s *UpdateFileSystemRouteTestSuite) TestUpdateFileSystemReturnsNoContentOnV

s.router.ServeHTTP(s.recorder, request)
s.Equal(http.StatusNoContent, s.recorder.Code)
s.runnerMock.AssertCalled(s.T(), "UpdateFileSystem", mock.AnythingOfType("*dto.UpdateFileSystemRequest"))
s.runnerMock.AssertCalled(s.T(), "UpdateFileSystem",
mock.AnythingOfType("*dto.UpdateFileSystemRequest"), mock.Anything)
}

func (s *UpdateFileSystemRouteTestSuite) TestUpdateFileSystemReturnsBadRequestOnInvalidRequestBody() {
Expand All @@ -314,7 +316,7 @@ func (s *UpdateFileSystemRouteTestSuite) TestUpdateFileSystemToNonExistingRunner

func (s *UpdateFileSystemRouteTestSuite) TestUpdateFileSystemReturnsInternalServerErrorWhenCopyFailed() {
s.runnerMock.
On("UpdateFileSystem", mock.AnythingOfType("*dto.UpdateFileSystemRequest")).
On("UpdateFileSystem", mock.AnythingOfType("*dto.UpdateFileSystemRequest"), mock.Anything).
Return(runner.ErrorFileCopyFailed)

copyRequest := dto.UpdateFileSystemRequest{}
Expand Down
16 changes: 9 additions & 7 deletions internal/api/websocket.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,14 @@ func (r *RunnerController) connectToRunner(writer http.ResponseWriter, request *
log.WithField("runnerId", targetRunner.ID()).
WithField("executionID", logging.RemoveNewlineSymbol(executionID)).
Info("Running execution")
exit, cancel, err := targetRunner.ExecuteInteractively(executionID,
proxy.Input, proxy.Output.StdOut(), proxy.Output.StdErr())
if err != nil {
log.WithError(err).Warn("Cannot execute request.")
return // The proxy is stopped by the defered cancel.
}
logging.StartSpan("api.runner.connect", "Execute Interactively", request.Context(), func(ctx context.Context) {
exit, cancel, err := targetRunner.ExecuteInteractively(executionID,
proxy.Input, proxy.Output.StdOut(), proxy.Output.StdErr(), ctx)
if err != nil {
log.WithError(err).Warn("Cannot execute request.")
return // The proxy is stopped by the deferred cancel.
}

proxy.waitForExit(exit, cancel)
proxy.waitForExit(exit, cancel)
})
}
4 changes: 3 additions & 1 deletion internal/environment/abstract_manager.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package environment

import (
"context"
"fmt"
"github.com/openHPI/poseidon/internal/runner"
"github.com/openHPI/poseidon/pkg/dto"
Expand Down Expand Up @@ -37,7 +38,8 @@ func (n *AbstractManager) Get(_ dto.EnvironmentID, _ bool) (runner.ExecutionEnvi
return nil, runner.ErrRunnerNotFound
}

func (n *AbstractManager) CreateOrUpdate(_ dto.EnvironmentID, _ dto.ExecutionEnvironmentRequest) (bool, error) {
func (n *AbstractManager) CreateOrUpdate(_ dto.EnvironmentID, _ dto.ExecutionEnvironmentRequest, _ context.Context) (
bool, error) {
return false, nil
}

Expand Down
5 changes: 3 additions & 2 deletions internal/environment/aws_manager.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package environment

import (
"context"
"fmt"
"github.com/openHPI/poseidon/internal/config"
"github.com/openHPI/poseidon/internal/runner"
Expand Down Expand Up @@ -41,9 +42,9 @@ func (a *AWSEnvironmentManager) Get(id dto.EnvironmentID, fetch bool) (runner.Ex
}

func (a *AWSEnvironmentManager) CreateOrUpdate(
id dto.EnvironmentID, request dto.ExecutionEnvironmentRequest) (bool, error) {
id dto.EnvironmentID, request dto.ExecutionEnvironmentRequest, ctx context.Context) (bool, error) {
if !isAWSEnvironment(request) {
isCreated, err := a.NextHandler().CreateOrUpdate(id, request)
isCreated, err := a.NextHandler().CreateOrUpdate(id, request, ctx)
if err != nil {
return false, fmt.Errorf("aws wrapped: %w", err)
}
Expand Down
10 changes: 6 additions & 4 deletions internal/environment/aws_manager_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package environment

import (
"context"
"github.com/openHPI/poseidon/internal/config"
"github.com/openHPI/poseidon/internal/runner"
"github.com/openHPI/poseidon/pkg/dto"
Expand All @@ -18,7 +19,8 @@ func TestAWSEnvironmentManager_CreateOrUpdate(t *testing.T) {

t.Run("can create default Java environment", func(t *testing.T) {
config.Config.AWS.Functions = []string{uniqueImage}
_, err := m.CreateOrUpdate(tests.AnotherEnvironmentIDAsInteger, dto.ExecutionEnvironmentRequest{Image: uniqueImage})
_, err := m.CreateOrUpdate(
tests.AnotherEnvironmentIDAsInteger, dto.ExecutionEnvironmentRequest{Image: uniqueImage}, context.Background())
assert.NoError(t, err)
})

Expand All @@ -31,14 +33,14 @@ func TestAWSEnvironmentManager_CreateOrUpdate(t *testing.T) {
t.Run("non-handleable requests are forwarded to the next manager", func(t *testing.T) {
nextHandler := &ManagerHandlerMock{}
nextHandler.On("CreateOrUpdate", mock.AnythingOfType("dto.EnvironmentID"),
mock.AnythingOfType("dto.ExecutionEnvironmentRequest")).Return(true, nil)
mock.AnythingOfType("dto.ExecutionEnvironmentRequest"), mock.Anything).Return(true, nil)
m.SetNextHandler(nextHandler)

request := dto.ExecutionEnvironmentRequest{}
_, err := m.CreateOrUpdate(tests.DefaultEnvironmentIDAsInteger, request)
_, err := m.CreateOrUpdate(tests.DefaultEnvironmentIDAsInteger, request, context.Background())
assert.NoError(t, err)
nextHandler.AssertCalled(t, "CreateOrUpdate",
dto.EnvironmentID(tests.DefaultEnvironmentIDAsInteger), request)
dto.EnvironmentID(tests.DefaultEnvironmentIDAsInteger), request, mock.Anything)
})
}

Expand Down
2 changes: 2 additions & 0 deletions internal/environment/manager.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package environment

import (
"context"
"github.com/openHPI/poseidon/internal/runner"
"github.com/openHPI/poseidon/pkg/dto"
)
Expand Down Expand Up @@ -30,6 +31,7 @@ type Manager interface {
CreateOrUpdate(
id dto.EnvironmentID,
request dto.ExecutionEnvironmentRequest,
ctx context.Context,
) (bool, error)

// Delete removes the specified execution environment.
Expand Down
22 changes: 12 additions & 10 deletions internal/environment/manager_handler_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading