Skip to content

Commit

Permalink
cleanup from pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
willmyrs committed Jan 31, 2025
1 parent b928076 commit 75de604
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 56 deletions.
3 changes: 3 additions & 0 deletions ecs-agent/introspection/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,20 +93,23 @@ func NewServer(agentState v1.AgentState, metricsFactory metrics.EntryFactory, op
}
return setup(agentState, metricsFactory, config)
}

func v1HandlersSetup(serverMux *http.ServeMux,
agentState v1.AgentState,
metricsFactory metrics.EntryFactory) {
serverMux.HandleFunc(handlers.V1AgentMetadataPath, handlers.AgentMetadataHandler(agentState, metricsFactory))
serverMux.HandleFunc(handlers.V1TasksMetadataPath, handlers.TasksMetadataHandler(agentState, metricsFactory))
serverMux.HandleFunc(licensePath, licenseHandler(agentState, metricsFactory))
}

func pprofHandlerSetup(serverMux *http.ServeMux) {
serverMux.HandleFunc(pprofBasePath, pprofIndexHandler)
serverMux.HandleFunc(pprofCMDLinePath, pprofCmdlineHandler)
serverMux.HandleFunc(pprofProfilePath, pprofProfileHandler)
serverMux.HandleFunc(pprofSymbolPath, pprofSymbolHandler)
serverMux.HandleFunc(pprofTracePath, pprofTraceHandler)
}

func setup(
agentState v1.AgentState,
metricsFactory metrics.EntryFactory,
Expand Down
2 changes: 1 addition & 1 deletion ecs-agent/introspection/server_integ_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func TestWriteTimeout(t *testing.T) {
return testAgentMetadata, nil
})

response, err := performRequest(t, mockAgentState, mockMetricsFactory, handlers.V1AgentMetadataPath, WithReadTimeout(time.Millisecond*150))
response, err := performRequest(t, mockAgentState, mockMetricsFactory, handlers.V1AgentMetadataPath, WithWriteTimeout(time.Millisecond*150))
require.NoError(t, err)

bodyBytes, err := io.ReadAll(response.Body)
Expand Down
34 changes: 34 additions & 0 deletions ecs-agent/introspection/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ import (
"github.com/stretchr/testify/require"
)

const (
writeTimeout = time.Second * 5
readTimeout = time.Second * 6
)

var runtimeStatsConfigForTest = false

func TestNewServerErrors(t *testing.T) {
Expand All @@ -46,6 +51,35 @@ func TestNewServerErrors(t *testing.T) {
})
}

func TestNewServerConfig(t *testing.T) {
t.Run("read/write timeouts", func(t *testing.T) {
ctrl := gomock.NewController(t)
agentState := mock_v1.NewMockAgentState(ctrl)
metricsFactory := mock_metrics.NewMockEntryFactory(ctrl)
server, err := NewServer(agentState, metricsFactory,
WithReadTimeout(readTimeout),
WithWriteTimeout(writeTimeout),
)
assert.Nil(t, err)
assert.Equal(t, readTimeout, server.ReadTimeout)
assert.Equal(t, writeTimeout, server.WriteTimeout)
})

t.Run("overwrite write timeout when profiling enabled", func(t *testing.T) {
ctrl := gomock.NewController(t)
agentState := mock_v1.NewMockAgentState(ctrl)
metricsFactory := mock_metrics.NewMockEntryFactory(ctrl)
server, err := NewServer(agentState, metricsFactory,
WithReadTimeout(readTimeout),
WithWriteTimeout(writeTimeout),
WithRuntimeStats(true),
)
assert.Nil(t, err)
assert.Equal(t, readTimeout, server.ReadTimeout)
assert.Equal(t, writeTimeoutForPprof, server.WriteTimeout)
})
}

func TestNewServerDefaults(t *testing.T) {
ctrl := gomock.NewController(t)
agentState := mock_v1.NewMockAgentState(ctrl)
Expand Down
65 changes: 10 additions & 55 deletions ecs-agent/introspection/v1/handlers/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,15 @@ func TasksMetadataHandler(
if dockerIDExists {
// Find the task that has a container with a matching docker ID.
if len(dockerID) > dockerShortIDLen {
getTaskByID(agentState, metricsFactory, dockerID, w)
getTaskMetadata(agentState.GetTaskMetadataByID, dockerID, field.DockerId, metricsFactory, w)
return
} else {
getTaskByShortID(agentState, metricsFactory, dockerID, w)
getTaskMetadata(agentState.GetTaskMetadataByShortID, dockerID, field.DockerId, metricsFactory, w)
return
}
} else if taskARNExists {
// Find the task with a matching Arn.
getTaskByARN(agentState, metricsFactory, taskArn, w)
getTaskMetadata(agentState.GetTaskMetadataByArn, taskArn, field.TaskARN, metricsFactory, w)
return
} else {
// Return all tasks.
Expand Down Expand Up @@ -137,63 +137,18 @@ func getTasksMetadata(
tmdsutils.WriteJSONResponse(w, http.StatusOK, tasksMetadata, requestTypeTasks)
}

// getTaskByARN writes metadata for the corresponding task to the response, or an
// error status if the agent cannot return the metadata
func getTaskByARN(
agentState v1.AgentState,
metricsFactory metrics.EntryFactory,
taskARN string,
w http.ResponseWriter,
) {
taskMetadata, err := agentState.GetTaskMetadataByArn(taskARN)
if err != nil {
logger.Error("Failed to get v1 task metadata.", logger.Fields{
field.Error: err,
field.TaskARN: taskARN,
})
responseCode, metricName := getHTTPErrorCode(err)
metricsFactory.New(metricName).Done(err)
tmdsutils.WriteJSONResponse(w, responseCode, v1.TaskResponse{}, requestTypeTasks)
return
}
tmdsutils.WriteJSONResponse(w, http.StatusOK, taskMetadata, requestTypeTasks)
}

// getTaskByID writes metadata for the corresponding task to the response, or an
// error status if the agent cannot return the metadata
func getTaskByID(
agentState v1.AgentState,
func getTaskMetadata(
lookupFn func(key string) (*v1.TaskResponse, error),
key string,
keyType string,
metricsFactory metrics.EntryFactory,
dockerID string,
w http.ResponseWriter,
) {
taskMetadata, err := agentState.GetTaskMetadataByID(dockerID)
taskMetadata, err := lookupFn(key)
if err != nil {
logger.Error("Failed to get v1 task metadata.", logger.Fields{
field.Error: err,
field.DockerId: dockerID,
})
responseCode, metricName := getHTTPErrorCode(err)
metricsFactory.New(metricName).Done(err)
tmdsutils.WriteJSONResponse(w, responseCode, v1.TaskResponse{}, requestTypeTasks)
return
}
tmdsutils.WriteJSONResponse(w, http.StatusOK, taskMetadata, requestTypeTasks)
}

// getTaskByShortID writes metadata for the corresponding task to the response, or an
// error status if the agent cannot return the metadata
func getTaskByShortID(
agentState v1.AgentState,
metricsFactory metrics.EntryFactory,
shortDockerID string,
w http.ResponseWriter,
) {
taskMetadata, err := agentState.GetTaskMetadataByShortID(shortDockerID)
if err != nil {
logger.Error("Failed to get v1 task metadata.", logger.Fields{
field.Error: err,
field.DockerId: shortDockerID,
field.Error: err,
keyType: key,
})
responseCode, metricName := getHTTPErrorCode(err)
metricsFactory.New(metricName).Done(err)
Expand Down

0 comments on commit 75de604

Please sign in to comment.