From 02e7ffd5b7a319d07fccd1a014964065f71e41ed Mon Sep 17 00:00:00 2001 From: Luis Neto Date: Wed, 11 Dec 2024 02:48:41 -0800 Subject: [PATCH] TUN-8792: Make diag/system endpoint always return a JSON ## Summary Change the system information collector and respective http handler so that it always returns a JSON. Closes [TUN-8792](https://jira.cfdata.org/browse/TUN-8792) --- diagnostic/client.go | 2 +- diagnostic/handlers.go | 30 +++---- diagnostic/handlers_test.go | 63 +++++--------- diagnostic/system_collector.go | 114 +++++++++++++++++++++---- diagnostic/system_collector_linux.go | 72 +++++++++++----- diagnostic/system_collector_macos.go | 74 ++++++++++++---- diagnostic/system_collector_windows.go | 72 +++++++++++----- metrics/metrics.go | 2 +- 8 files changed, 288 insertions(+), 141 deletions(-) diff --git a/diagnostic/client.go b/diagnostic/client.go index 0e82a4c6..6e4dc2d3 100644 --- a/diagnostic/client.go +++ b/diagnostic/client.go @@ -141,7 +141,7 @@ func (client *httpClient) GetSystemInformation(ctx context.Context, writer io.Wr return err } - return copyToWriter(response, writer) + return copyJSONToWriter(response, writer) } func (client *httpClient) GetMetrics(ctx context.Context, writer io.Writer) error { diff --git a/diagnostic/handlers.go b/diagnostic/handlers.go index 1d9ef4f6..e4d85db4 100644 --- a/diagnostic/handlers.go +++ b/diagnostic/handlers.go @@ -59,6 +59,11 @@ func (handler *Handler) InstallEndpoints(router *http.ServeMux) { router.HandleFunc(systemInformationEndpoint, handler.SystemHandler) } +type SystemInformationResponse struct { + Info *SystemInformation `json:"info"` + Err error `json:"errors"` +} + func (handler *Handler) SystemHandler(writer http.ResponseWriter, request *http.Request) { logger := handler.log.With().Str(collectorField, systemCollectorName).Logger() logger.Info().Msg("Collection started") @@ -69,30 +74,15 @@ func (handler *Handler) SystemHandler(writer http.ResponseWriter, request *http. defer cancel() - info, rawInfo, err := handler.systemCollector.Collect(ctx) - if err != nil { - logger.Error().Err(err).Msg("error occurred whilst collecting system information") + info, err := handler.systemCollector.Collect(ctx) - if rawInfo != "" { - logger.Info().Msg("using raw information fallback") - bytes := []byte(rawInfo) - writeResponse(writer, bytes, &logger) - } else { - logger.Error().Msg("no raw information available") - writer.WriteHeader(http.StatusInternalServerError) - } - - return - } - - if info == nil { - logger.Error().Msgf("system information collection is nil") - writer.WriteHeader(http.StatusInternalServerError) + response := SystemInformationResponse{ + Info: info, + Err: err, } encoder := json.NewEncoder(writer) - - err = encoder.Encode(info) + err = encoder.Encode(response) if err != nil { logger.Error().Err(err).Msgf("error occurred whilst serializing information") writer.WriteHeader(http.StatusInternalServerError) diff --git a/diagnostic/handlers_test.go b/diagnostic/handlers_test.go index 3a300bff..2849241c 100644 --- a/diagnostic/handlers_test.go +++ b/diagnostic/handlers_test.go @@ -4,10 +4,10 @@ import ( "context" "encoding/json" "errors" - "io" "net" "net/http" "net/http/httptest" + "runtime" "testing" "github.com/google/uuid" @@ -20,11 +20,13 @@ import ( "github.com/cloudflare/cloudflared/tunnelstate" ) -type SystemCollectorMock struct{} +type SystemCollectorMock struct { + systemInfo *diagnostic.SystemInformation + err error +} const ( systemInformationKey = "sikey" - rawInformationKey = "rikey" errorKey = "errkey" ) @@ -46,24 +48,8 @@ func newTrackerFromConns(t *testing.T, connections []tunnelstate.IndexedConnecti return tracker } -func setCtxValuesForSystemCollector( - systemInfo *diagnostic.SystemInformation, - rawInfo string, - err error, -) context.Context { - ctx := context.Background() - ctx = context.WithValue(ctx, systemInformationKey, systemInfo) - ctx = context.WithValue(ctx, rawInformationKey, rawInfo) - ctx = context.WithValue(ctx, errorKey, err) - - return ctx -} - -func (*SystemCollectorMock) Collect(ctx context.Context) (*diagnostic.SystemInformation, string, error) { - si, _ := ctx.Value(systemInformationKey).(*diagnostic.SystemInformation) - ri, _ := ctx.Value(rawInformationKey).(string) - err, _ := ctx.Value(errorKey).(error) - return si, ri, err +func (collector *SystemCollectorMock) Collect(context.Context) (*diagnostic.SystemInformation, error) { + return collector.systemInfo, collector.err } func TestSystemHandler(t *testing.T) { @@ -73,7 +59,6 @@ func TestSystemHandler(t *testing.T) { tests := []struct { name string systemInfo *diagnostic.SystemInformation - rawInfo string err error statusCode int }{ @@ -82,47 +67,39 @@ func TestSystemHandler(t *testing.T) { systemInfo: diagnostic.NewSystemInformation( 0, 0, 0, 0, "string", "string", "string", "string", - "string", "string", nil, + "string", "string", + runtime.Version(), runtime.GOARCH, nil, ), - rawInfo: "", + err: nil, statusCode: http.StatusOK, }, - { - name: "on error and raw info", systemInfo: nil, - rawInfo: "raw info", err: errors.New("an error"), statusCode: http.StatusOK, - }, { name: "on error and no raw info", systemInfo: nil, - rawInfo: "", err: errors.New("an error"), statusCode: http.StatusInternalServerError, - }, - { - name: "malformed response", systemInfo: nil, rawInfo: "", err: nil, statusCode: http.StatusInternalServerError, + err: errors.New("an error"), statusCode: http.StatusOK, }, } for _, tCase := range tests { t.Run(tCase.name, func(t *testing.T) { t.Parallel() - handler := diagnostic.NewDiagnosticHandler(&log, 0, &SystemCollectorMock{}, uuid.New(), uuid.New(), nil, map[string]string{}, nil) + handler := diagnostic.NewDiagnosticHandler(&log, 0, &SystemCollectorMock{ + systemInfo: tCase.systemInfo, + err: tCase.err, + }, uuid.New(), uuid.New(), nil, map[string]string{}, nil) recorder := httptest.NewRecorder() - ctx := setCtxValuesForSystemCollector(tCase.systemInfo, tCase.rawInfo, tCase.err) - request, err := http.NewRequestWithContext(ctx, http.MethodGet, "/diag/syste,", nil) + ctx := context.Background() + request, err := http.NewRequestWithContext(ctx, http.MethodGet, "/diag/system", nil) require.NoError(t, err) handler.SystemHandler(recorder, request) assert.Equal(t, tCase.statusCode, recorder.Code) if tCase.statusCode == http.StatusOK && tCase.systemInfo != nil { - var response diagnostic.SystemInformation - + var response diagnostic.SystemInformationResponse decoder := json.NewDecoder(recorder.Body) - err = decoder.Decode(&response) + err := decoder.Decode(&response) require.NoError(t, err) - assert.Equal(t, tCase.systemInfo, &response) - } else if tCase.statusCode == http.StatusOK && tCase.rawInfo != "" { - rawBytes, err := io.ReadAll(recorder.Body) - require.NoError(t, err) - assert.Equal(t, tCase.rawInfo, string(rawBytes)) + assert.Equal(t, tCase.systemInfo, response.Info) } }) } diff --git a/diagnostic/system_collector.go b/diagnostic/system_collector.go index 08f2a47f..96e51231 100644 --- a/diagnostic/system_collector.go +++ b/diagnostic/system_collector.go @@ -1,6 +1,82 @@ package diagnostic -import "context" +import ( + "context" + "encoding/json" + "errors" + "strings" +) + +type SystemInformationError struct { + Err error `json:"error"` + RawInfo string `json:"rawInfo"` +} + +func (err SystemInformationError) Error() string { + return err.Err.Error() +} + +func (err SystemInformationError) MarshalJSON() ([]byte, error) { + s := map[string]string{ + "error": err.Err.Error(), + "rawInfo": err.RawInfo, + } + + return json.Marshal(s) +} + +type SystemInformationGeneralError struct { + OperatingSystemInformationError error + MemoryInformationError error + FileDescriptorsInformationError error + DiskVolumeInformationError error +} + +func (err SystemInformationGeneralError) Error() string { + builder := &strings.Builder{} + builder.WriteString("errors found:") + + if err.OperatingSystemInformationError != nil { + builder.WriteString(err.OperatingSystemInformationError.Error() + ", ") + } + + if err.MemoryInformationError != nil { + builder.WriteString(err.MemoryInformationError.Error() + ", ") + } + + if err.FileDescriptorsInformationError != nil { + builder.WriteString(err.FileDescriptorsInformationError.Error() + ", ") + } + + if err.DiskVolumeInformationError != nil { + builder.WriteString(err.DiskVolumeInformationError.Error() + ", ") + } + + return builder.String() +} + +func (err SystemInformationGeneralError) MarshalJSON() ([]byte, error) { + data := map[string]SystemInformationError{} + + var sysErr SystemInformationError + if errors.As(err.OperatingSystemInformationError, &sysErr) { + data["operatingSystemInformationError"] = sysErr + } + + if errors.As(err.MemoryInformationError, &sysErr) { + data["memoryInformationError"] = sysErr + } + + if errors.As(err.FileDescriptorsInformationError, &sysErr) { + data["fileDescriptorsInformationError"] = sysErr + } + + if errors.As(err.DiskVolumeInformationError, &sysErr) { + data["diskVolumeInformationError"] = sysErr + } + + return json.Marshal(data) +} type DiskVolumeInformation struct { Name string `json:"name"` // represents the filesystem in linux/macos or device name in windows @@ -17,17 +93,19 @@ func NewDiskVolumeInformation(name string, maximum, current uint64) *DiskVolumeI } type SystemInformation struct { - MemoryMaximum uint64 `json:"memoryMaximum"` // represents the maximum memory of the system in kilobytes - MemoryCurrent uint64 `json:"memoryCurrent"` // represents the system's memory in use in kilobytes - FileDescriptorMaximum uint64 `json:"fileDescriptorMaximum"` // represents the maximum number of file descriptors of the system - FileDescriptorCurrent uint64 `json:"fileDescriptorCurrent"` // represents the system's file descriptors in use - OsSystem string `json:"osSystem"` // represents the operating system name i.e.: linux, windows, darwin - HostName string `json:"hostName"` // represents the system host name - OsVersion string `json:"osVersion"` // detailed information about the system's release version level - OsRelease string `json:"osRelease"` // detailed information about the system's release - Architecture string `json:"architecture"` // represents the system's hardware platform i.e: arm64/amd64 - CloudflaredVersion string `json:"cloudflaredVersion"` // the runtime version of cloudflared - Disk []*DiskVolumeInformation `json:"disk"` + MemoryMaximum uint64 `json:"memoryMaximum,omitempty"` // represents the maximum memory of the system in kilobytes + MemoryCurrent uint64 `json:"memoryCurrent,omitempty"` // represents the system's memory in use in kilobytes + FileDescriptorMaximum uint64 `json:"fileDescriptorMaximum,omitempty"` // represents the maximum number of file descriptors of the system + FileDescriptorCurrent uint64 `json:"fileDescriptorCurrent,omitempty"` // represents the system's file descriptors in use + OsSystem string `json:"osSystem,omitempty"` // represents the operating system name i.e.: linux, windows, darwin + HostName string `json:"hostName,omitempty"` // represents the system host name + OsVersion string `json:"osVersion,omitempty"` // detailed information about the system's release version level + OsRelease string `json:"osRelease,omitempty"` // detailed information about the system's release + Architecture string `json:"architecture,omitempty"` // represents the system's hardware platform i.e: arm64/amd64 + CloudflaredVersion string `json:"cloudflaredVersion,omitempty"` // the runtime version of cloudflared + GoVersion string `json:"goVersion,omitempty"` + GoArch string `json:"goArch,omitempty"` + Disk []*DiskVolumeInformation `json:"disk,omitempty"` } func NewSystemInformation( @@ -40,7 +118,9 @@ func NewSystemInformation( osVersion, osRelease, architecture, - cloudflaredVersion string, + cloudflaredVersion, + goVersion, + goArchitecture string, disk []*DiskVolumeInformation, ) *SystemInformation { return &SystemInformation{ @@ -54,17 +134,17 @@ func NewSystemInformation( osRelease, architecture, cloudflaredVersion, + goVersion, + goArchitecture, disk, } } type SystemCollector interface { // If the collection is successful it will return `SystemInformation` struct, - // an empty string, and a nil error. - // In case there is an error a string with the raw data will be returned - // however the returned string not contain all the data points. + // and a nil error. // // This function expects that the caller sets the context timeout to prevent // long-lived collectors. - Collect(ctx context.Context) (*SystemInformation, string, error) + Collect(ctx context.Context) (*SystemInformation, error) } diff --git a/diagnostic/system_collector_linux.go b/diagnostic/system_collector_linux.go index 35d3cc9b..49c0e6c2 100644 --- a/diagnostic/system_collector_linux.go +++ b/diagnostic/system_collector_linux.go @@ -6,6 +6,7 @@ import ( "context" "fmt" "os/exec" + "runtime" "strconv" "strings" ) @@ -22,45 +23,74 @@ func NewSystemCollectorImpl( } } -func (collector *SystemCollectorImpl) Collect(ctx context.Context) (*SystemInformation, string, error) { +func (collector *SystemCollectorImpl) Collect(ctx context.Context) (*SystemInformation, error) { memoryInfo, memoryInfoRaw, memoryInfoErr := collectMemoryInformation(ctx) fdInfo, fdInfoRaw, fdInfoErr := collectFileDescriptorInformation(ctx) disks, disksRaw, diskErr := collectDiskVolumeInformationUnix(ctx) osInfo, osInfoRaw, osInfoErr := collectOSInformationUnix(ctx) + var memoryMaximum, memoryCurrent, fileDescriptorMaximum, fileDescriptorCurrent uint64 + var osSystem, name, osVersion, osRelease, architecture string + gerror := SystemInformationGeneralError{} + if memoryInfoErr != nil { - raw := RawSystemInformation(osInfoRaw, memoryInfoRaw, fdInfoRaw, disksRaw) - return nil, raw, memoryInfoErr + gerror.MemoryInformationError = SystemInformationError{ + Err: memoryInfoErr, + RawInfo: memoryInfoRaw, + } + } else { + memoryMaximum = memoryInfo.MemoryMaximum + memoryCurrent = memoryInfo.MemoryCurrent } if fdInfoErr != nil { - raw := RawSystemInformation(osInfoRaw, memoryInfoRaw, fdInfoRaw, disksRaw) - return nil, raw, fdInfoErr + gerror.FileDescriptorsInformationError = SystemInformationError{ + Err: fdInfoErr, + RawInfo: fdInfoRaw, + } + } else { + fileDescriptorMaximum = fdInfo.FileDescriptorMaximum + fileDescriptorCurrent = fdInfo.FileDescriptorCurrent } if diskErr != nil { - raw := RawSystemInformation(osInfoRaw, memoryInfoRaw, fdInfoRaw, disksRaw) - return nil, raw, diskErr + gerror.DiskVolumeInformationError = SystemInformationError{ + Err: diskErr, + RawInfo: disksRaw, + } } if osInfoErr != nil { - raw := RawSystemInformation(osInfoRaw, memoryInfoRaw, fdInfoRaw, disksRaw) - return nil, raw, osInfoErr + gerror.OperatingSystemInformationError = SystemInformationError{ + Err: osInfoErr, + RawInfo: osInfoRaw, + } + } else { + osSystem = osInfo.OsSystem + name = osInfo.Name + osVersion = osInfo.OsVersion + osRelease = osInfo.OsRelease + architecture = osInfo.Architecture } - return NewSystemInformation( - memoryInfo.MemoryMaximum, - memoryInfo.MemoryCurrent, - fdInfo.FileDescriptorMaximum, - fdInfo.FileDescriptorCurrent, - osInfo.OsSystem, - osInfo.Name, - osInfo.OsVersion, - osInfo.OsRelease, - osInfo.Architecture, - collector.version, + cloudflaredVersion := collector.version + info := NewSystemInformation( + memoryMaximum, + memoryCurrent, + fileDescriptorMaximum, + fileDescriptorCurrent, + osSystem, + name, + osVersion, + osRelease, + architecture, + cloudflaredVersion, + runtime.Version(), + runtime.GOARCH, disks, - ), "", nil + ) + + return info, gerror } func collectMemoryInformation(ctx context.Context) (*MemoryInformation, string, error) { diff --git a/diagnostic/system_collector_macos.go b/diagnostic/system_collector_macos.go index 9ec96cfd..2f5ac740 100644 --- a/diagnostic/system_collector_macos.go +++ b/diagnostic/system_collector_macos.go @@ -6,6 +6,7 @@ import ( "context" "fmt" "os/exec" + "runtime" "strconv" ) @@ -21,41 +22,80 @@ func NewSystemCollectorImpl( } } -func (collector *SystemCollectorImpl) Collect(ctx context.Context) (*SystemInformation, string, error) { +func (collector *SystemCollectorImpl) Collect(ctx context.Context) (*SystemInformation, error) { memoryInfo, memoryInfoRaw, memoryInfoErr := collectMemoryInformation(ctx) fdInfo, fdInfoRaw, fdInfoErr := collectFileDescriptorInformation(ctx) disks, disksRaw, diskErr := collectDiskVolumeInformationUnix(ctx) osInfo, osInfoRaw, osInfoErr := collectOSInformationUnix(ctx) + var memoryMaximum, memoryCurrent, fileDescriptorMaximum, fileDescriptorCurrent uint64 + var osSystem, name, osVersion, osRelease, architecture string + + err := SystemInformationGeneralError{ + OperatingSystemInformationError: nil, + MemoryInformationError: nil, + FileDescriptorsInformationError: nil, + DiskVolumeInformationError: nil, + } + if memoryInfoErr != nil { - return nil, RawSystemInformation(osInfoRaw, memoryInfoRaw, fdInfoRaw, disksRaw), memoryInfoErr + err.MemoryInformationError = SystemInformationError{ + Err: memoryInfoErr, + RawInfo: memoryInfoRaw, + } + } else { + memoryMaximum = memoryInfo.MemoryMaximum + memoryCurrent = memoryInfo.MemoryCurrent } if fdInfoErr != nil { - return nil, RawSystemInformation(osInfoRaw, memoryInfoRaw, fdInfoRaw, disksRaw), fdInfoErr + err.FileDescriptorsInformationError = SystemInformationError{ + Err: fdInfoErr, + RawInfo: fdInfoRaw, + } + } else { + fileDescriptorMaximum = fdInfo.FileDescriptorMaximum + fileDescriptorCurrent = fdInfo.FileDescriptorCurrent } if diskErr != nil { - return nil, RawSystemInformation(osInfoRaw, memoryInfoRaw, fdInfoRaw, disksRaw), diskErr + err.DiskVolumeInformationError = SystemInformationError{ + Err: diskErr, + RawInfo: disksRaw, + } } if osInfoErr != nil { - return nil, RawSystemInformation(osInfoRaw, memoryInfoRaw, fdInfoRaw, disksRaw), osInfoErr + err.OperatingSystemInformationError = SystemInformationError{ + Err: osInfoErr, + RawInfo: osInfoRaw, + } + } else { + osSystem = osInfo.OsSystem + name = osInfo.Name + osVersion = osInfo.OsVersion + osRelease = osInfo.OsRelease + architecture = osInfo.Architecture } - return NewSystemInformation( - memoryInfo.MemoryMaximum, - memoryInfo.MemoryCurrent, - fdInfo.FileDescriptorMaximum, - fdInfo.FileDescriptorCurrent, - osInfo.OsSystem, - osInfo.Name, - osInfo.OsVersion, - osInfo.OsRelease, - osInfo.Architecture, - collector.version, + cloudflaredVersion := collector.version + info := NewSystemInformation( + memoryMaximum, + memoryCurrent, + fileDescriptorMaximum, + fileDescriptorCurrent, + osSystem, + name, + osVersion, + osRelease, + architecture, + cloudflaredVersion, + runtime.Version(), + runtime.GOARCH, disks, - ), "", nil + ) + + return info, err } func collectFileDescriptorInformation(ctx context.Context) ( diff --git a/diagnostic/system_collector_windows.go b/diagnostic/system_collector_windows.go index 0866a739..41eaafcc 100644 --- a/diagnostic/system_collector_windows.go +++ b/diagnostic/system_collector_windows.go @@ -6,6 +6,7 @@ import ( "context" "fmt" "os/exec" + "runtime" "strconv" ) @@ -22,41 +23,70 @@ func NewSystemCollectorImpl( version, } } -func (collector *SystemCollectorImpl) Collect(ctx context.Context) (*SystemInformation, string, error) { + +func (collector *SystemCollectorImpl) Collect(ctx context.Context) (*SystemInformation, error) { memoryInfo, memoryInfoRaw, memoryInfoErr := collectMemoryInformation(ctx) disks, disksRaw, diskErr := collectDiskVolumeInformation(ctx) osInfo, osInfoRaw, osInfoErr := collectOSInformation(ctx) + var memoryMaximum, memoryCurrent, fileDescriptorMaximum, fileDescriptorCurrent uint64 + var osSystem, name, osVersion, osRelease, architecture string + + err := SystemInformationGeneralError{ + OperatingSystemInformationError: nil, + MemoryInformationError: nil, + FileDescriptorsInformationError: nil, + DiskVolumeInformationError: nil, + } + if memoryInfoErr != nil { - raw := RawSystemInformation(osInfoRaw, memoryInfoRaw, "", disksRaw) - return nil, raw, memoryInfoErr + err.MemoryInformationError = SystemInformationError{ + Err: memoryInfoErr, + RawInfo: memoryInfoRaw, + } + } else { + memoryMaximum = memoryInfo.MemoryMaximum + memoryCurrent = memoryInfo.MemoryCurrent } if diskErr != nil { - raw := RawSystemInformation(osInfoRaw, memoryInfoRaw, "", disksRaw) - return nil, raw, diskErr + err.DiskVolumeInformationError = SystemInformationError{ + Err: diskErr, + RawInfo: disksRaw, + } } if osInfoErr != nil { - raw := RawSystemInformation(osInfoRaw, memoryInfoRaw, "", disksRaw) - return nil, raw, osInfoErr + err.OperatingSystemInformationError = SystemInformationError{ + Err: osInfoErr, + RawInfo: osInfoRaw, + } + } else { + osSystem = osInfo.OsSystem + name = osInfo.Name + osVersion = osInfo.OsVersion + osRelease = osInfo.OsRelease + architecture = osInfo.Architecture } - return NewSystemInformation( - memoryInfo.MemoryMaximum, - memoryInfo.MemoryCurrent, - // For windows we leave both the fileDescriptorMaximum and fileDescriptorCurrent with zero - // since there is no obvious way to get this information. - 0, - 0, - osInfo.OsSystem, - osInfo.Name, - osInfo.OsVersion, - osInfo.OsRelease, - osInfo.Architecture, - collector.version, + cloudflaredVersion := collector.version + info := NewSystemInformation( + memoryMaximum, + memoryCurrent, + fileDescriptorMaximum, + fileDescriptorCurrent, + osSystem, + name, + osVersion, + osRelease, + architecture, + cloudflaredVersion, + runtime.Version(), + runtime.GOARCH, disks, - ), "", nil + ) + + return info, err } func collectMemoryInformation(ctx context.Context) (*MemoryInformation, string, error) { diff --git a/metrics/metrics.go b/metrics/metrics.go index c4d3abb6..2a4fe993 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -29,7 +29,7 @@ var Runtime = "host" func GetMetricsDefaultAddress(runtimeType string) string { // When issuing the diagnostic command we may have to reach a server that is - // running in a virtual enviroment and in that case we must bind to 0.0.0.0 + // running in a virtual environment and in that case we must bind to 0.0.0.0 // otherwise the server won't be reachable. switch runtimeType { case "virtual":