diff --git a/diagnostics/collection.go b/diagnostics/collection.go index 40e42e5b7..f3361c564 100644 --- a/diagnostics/collection.go +++ b/diagnostics/collection.go @@ -32,7 +32,8 @@ func Init(instanceID uuid.UUID) { if enabled { panic("already initialized") } - if instanceID.String() == "" { + if str := instanceID.String(); str == "" || + instanceID.String() == "00000000-0000-0000-0000-000000000000" { panic("empty UUID") } instanceUUID = instanceID diff --git a/diagnostics/collection_test.go b/diagnostics/collection_test.go new file mode 100644 index 000000000..e895a0a4e --- /dev/null +++ b/diagnostics/collection_test.go @@ -0,0 +1,109 @@ +// Copyright 2015 Light Code Labs, LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package diagnostics + +import ( + "fmt" + "testing" + + "github.com/google/uuid" +) + +func TestInit(t *testing.T) { + reset() + + id := doInit(t) // should not panic + + defer func() { + if r := recover(); r == nil { + t.Errorf("Second call to Init should have panicked") + } + }() + Init(id) // should panic +} + +func TestInitEmptyUUID(t *testing.T) { + reset() + defer func() { + if r := recover(); r == nil { + t.Errorf("Call to Init with empty UUID should have panicked") + } + }() + Init(uuid.UUID([16]byte{})) +} + +func TestSet(t *testing.T) { + reset() + + // should be no-op since we haven't called Init() yet + Set("test1", "foobar") + if _, ok := buffer["test"]; ok { + t.Errorf("Should not have inserted item when not initialized") + } + + // should work after we've initialized + doInit(t) + Set("test1", "foobar") + val, ok := buffer["test1"] + if !ok { + t.Errorf("Expected value to be in buffer, but it wasn't") + } else if val.(string) != "foobar" { + t.Errorf("Expected 'foobar', got '%v'", val) + } + + // should not overfill buffer + maxBufferItemsTmp := maxBufferItems + maxBufferItems = 10 + for i := 0; i < maxBufferItems+1; i++ { + Set(fmt.Sprintf("overfill_%d", i), "foobar") + } + if len(buffer) > maxBufferItems { + t.Errorf("Should not exceed max buffer size (%d); has %d items", + maxBufferItems, len(buffer)) + } + maxBufferItems = maxBufferItemsTmp + + // Should overwrite values + Set("test1", "foobar2") + val, ok = buffer["test1"] + if !ok { + t.Errorf("Expected value to be in buffer, but it wasn't") + } else if val.(string) != "foobar2" { + t.Errorf("Expected 'foobar2', got '%v'", val) + } +} + +// doInit calls Init() with a valid UUID +// and returns it. +func doInit(t *testing.T) uuid.UUID { + id, err := uuid.Parse(testUUID) + if err != nil { + t.Fatalf("Could not make UUID: %v", err) + } + Init(id) + return id +} + +// reset resets all the lovely package-level state; +// can be used as a set up function in tests. +func reset() { + instanceUUID = uuid.UUID{} + buffer = make(map[string]interface{}) + bufferItemCount = 0 + updating = false + enabled = false +} + +const testUUID = "0b6cfa22-0d4c-11e8-b11b-7a0058e13201" diff --git a/diagnostics/diagnostics.go b/diagnostics/diagnostics.go index dd4cac87d..2d6be6e70 100644 --- a/diagnostics/diagnostics.go +++ b/diagnostics/diagnostics.go @@ -216,32 +216,39 @@ type Payload struct { Data map[string]interface{} `json:"data,omitempty"` } -// httpClient should be used for HTTP requests. It -// is configured with a timeout for reliability. -var httpClient = http.Client{Timeout: 1 * time.Minute} +var ( + // httpClient should be used for HTTP requests. It + // is configured with a timeout for reliability. + httpClient = http.Client{Timeout: 1 * time.Minute} -// buffer holds the data that we are building up to send. -var buffer = make(map[string]interface{}) -var bufferItemCount = 0 -var bufferMu sync.RWMutex // protects both the buffer and its count + // buffer holds the data that we are building up to send. + buffer = make(map[string]interface{}) + bufferItemCount = 0 + bufferMu sync.RWMutex // protects both the buffer and its count -// updating is used to ensure only one -// update happens at a time. -var updating bool -var updateMu sync.Mutex + // updating is used to ensure only one + // update happens at a time. + updating bool + updateMu sync.Mutex -// updateTimer fires off the next update. -// If no update is scheduled, this is nil. -var updateTimer *time.Timer -var updateTimerMu sync.Mutex + // updateTimer fires off the next update. + // If no update is scheduled, this is nil. + updateTimer *time.Timer + updateTimerMu sync.Mutex -// instanceUUID is the ID of the current instance. -// This MUST be set to emit diagnostics. -var instanceUUID uuid.UUID + // instanceUUID is the ID of the current instance. + // This MUST be set to emit diagnostics. + instanceUUID uuid.UUID -// enabled indicates whether the package has -// been initialized and can be actively used. -var enabled bool + // enabled indicates whether the package has + // been initialized and can be actively used. + enabled bool + + // maxBufferItems is the maximum number of items we'll allow + // in the buffer before we start dropping new ones, in a + // rough (simple) attempt to keep memory use under control. + maxBufferItems = 100000 +) const ( // endpoint is the base URL to remote diagnostics server; @@ -255,9 +262,4 @@ const ( // this value should be a long duration to help alleviate // extra load on the server. defaultUpdateInterval = 1 * time.Hour - - // maxBufferItems is the maximum number of items we'll allow - // in the buffer before we start dropping new ones, in a - // rough (simple) attempt to keep memory use under control. - maxBufferItems = 100000 ) diff --git a/diagnostics/diagnostics_test.go b/diagnostics/diagnostics_test.go new file mode 100644 index 000000000..af458d99b --- /dev/null +++ b/diagnostics/diagnostics_test.go @@ -0,0 +1,59 @@ +// Copyright 2015 Light Code Labs, LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package diagnostics + +import ( + "encoding/json" + "testing" +) + +func TestMakePayloadAndResetBuffer(t *testing.T) { + reset() + id := doInit(t) + + buffer = map[string]interface{}{ + "foo1": "bar1", + "foo2": "bar2", + } + bufferItemCount = 2 + + payloadBytes, err := makePayloadAndResetBuffer() + if err != nil { + t.Fatalf("Error making payload bytes: %v", err) + } + + if len(buffer) != 0 { + t.Errorf("Expected buffer len to be 0, got %d", len(buffer)) + } + if bufferItemCount != 0 { + t.Errorf("Expected buffer item count to be 0, got %d", bufferItemCount) + } + + var payload Payload + err = json.Unmarshal(payloadBytes, &payload) + if err != nil { + t.Fatalf("Error deserializing payload: %v", err) + } + + if payload.InstanceID != id.String() { + t.Errorf("Expected instance ID to be set to '%s' but got '%s'", testUUID, payload.InstanceID) + } + if payload.Data == nil { + t.Errorf("Expected data to be set, but was nil") + } + if payload.Timestamp.IsZero() { + t.Errorf("Expected timestamp to be set, but was zero value") + } +}