From 0acbbc26e206e2c08c085d2d68fba27cd07ca2fa Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Mon, 10 Jan 2022 09:27:38 +0000 Subject: [PATCH] telemetry: Only send requests if data has changed --- internal/langserver/handlers/hooks_module.go | 143 +++++++++++-------- internal/state/installed_providers.go | 26 ++++ internal/state/installed_providers_test.go | 80 +++++++++++ internal/state/module.go | 8 +- 4 files changed, 197 insertions(+), 60 deletions(-) create mode 100644 internal/state/installed_providers.go create mode 100644 internal/state/installed_providers_test.go diff --git a/internal/langserver/handlers/hooks_module.go b/internal/langserver/handlers/hooks_module.go index 7d1e3d2b1..1ef658229 100644 --- a/internal/langserver/handlers/hooks_module.go +++ b/internal/langserver/handlers/hooks_module.go @@ -12,86 +12,117 @@ import ( ) func sendModuleTelemetry(ctx context.Context, store *state.StateStore, telemetrySender telemetry.Sender) state.ModuleChangeHook { - return func(_, newMod *state.Module) { + return func(oldMod, newMod *state.Module) { if newMod == nil { // module is being removed // TODO: Track module removal as an event return } - modId, err := store.GetModuleID(newMod.Path) - if err != nil { + properties, hasChanged := moduleTelemetryData(oldMod, newMod, store) + if !hasChanged { + // avoid sending telemetry if nothing has changed return } - properties := map[string]interface{}{ - "moduleId": modId, - } + telemetrySender.SendEvent(ctx, "moduleData", properties) + } +} - if len(newMod.Meta.CoreRequirements) > 0 { - properties["tfRequirements"] = newMod.Meta.CoreRequirements.String() - } - if newMod.Meta.Backend != nil { - properties["backend"] = newMod.Meta.Backend.Type - if data, ok := newMod.Meta.Backend.Data.(*backend.Remote); ok { - hostname := data.Hostname - - // anonymize any non-default hostnames - if hostname != "" && hostname != "app.terraform.io" { - hostname = "custom-hostname" - } +func moduleTelemetryData(oldMod, newMod *state.Module, store *state.StateStore) (map[string]interface{}, bool) { + properties := make(map[string]interface{}) + hasChanged := false - properties["backend.remote.hostname"] = hostname + if oldMod == nil || !oldMod.Meta.CoreRequirements.Equals(newMod.Meta.CoreRequirements) { + hasChanged = true + } + if len(newMod.Meta.CoreRequirements) > 0 { + properties["tfRequirements"] = newMod.Meta.CoreRequirements.String() + } + + if oldMod == nil || !oldMod.Meta.Backend.Equals(newMod.Meta.Backend) { + hasChanged = true + } + if newMod.Meta.Backend != nil { + properties["backend"] = newMod.Meta.Backend.Type + if data, ok := newMod.Meta.Backend.Data.(*backend.Remote); ok { + hostname := data.Hostname + + // anonymize any non-default hostnames + if hostname != "" && hostname != "app.terraform.io" { + hostname = "custom-hostname" } + + properties["backend.remote.hostname"] = hostname } - if len(newMod.Meta.ProviderRequirements) > 0 { - reqs := make(map[string]string, 0) - for pAddr, cons := range newMod.Meta.ProviderRequirements { - if telemetry.IsPublicProvider(pAddr) { - reqs[pAddr.String()] = cons.String() - continue - } + } - // anonymize any unknown providers or the ones not publicly listed - id, err := store.GetProviderID(pAddr) - if err != nil { - continue - } - addr := fmt.Sprintf("unlisted/%s", id) - reqs[addr] = cons.String() + if oldMod == nil || !oldMod.Meta.ProviderRequirements.Equals(newMod.Meta.ProviderRequirements) { + hasChanged = true + } + if len(newMod.Meta.ProviderRequirements) > 0 { + reqs := make(map[string]string, 0) + for pAddr, cons := range newMod.Meta.ProviderRequirements { + if telemetry.IsPublicProvider(pAddr) { + reqs[pAddr.String()] = cons.String() + continue } - properties["providerRequirements"] = reqs - } - if newMod.TerraformVersion != nil { - properties["tfVersion"] = newMod.TerraformVersion.String() + // anonymize any unknown providers or the ones not publicly listed + id, err := store.GetProviderID(pAddr) + if err != nil { + continue + } + addr := fmt.Sprintf("unlisted/%s", id) + reqs[addr] = cons.String() } + properties["providerRequirements"] = reqs + } - if len(newMod.InstalledProviders) > 0 { - installedProviders := make(map[string]string, 0) - for pAddr, pv := range newMod.InstalledProviders { - if telemetry.IsPublicProvider(pAddr) { - versionString := "" - if pv != nil { - versionString = pv.String() - } - installedProviders[pAddr.String()] = versionString - continue - } + if oldMod == nil || !oldMod.TerraformVersion.Equal(newMod.TerraformVersion) { + hasChanged = true + } + if newMod.TerraformVersion != nil { + properties["tfVersion"] = newMod.TerraformVersion.String() + } - // anonymize any unknown providers or the ones not publicly listed - id, err := store.GetProviderID(pAddr) - if err != nil { - continue + if oldMod == nil || !oldMod.InstalledProviders.Equals(newMod.InstalledProviders) { + hasChanged = true + } + if len(newMod.InstalledProviders) > 0 { + installedProviders := make(map[string]string, 0) + for pAddr, pv := range newMod.InstalledProviders { + if telemetry.IsPublicProvider(pAddr) { + versionString := "" + if pv != nil { + versionString = pv.String() } - addr := fmt.Sprintf("unlisted/%s", id) - installedProviders[addr] = "" + installedProviders[pAddr.String()] = versionString + continue } - properties["installedProviders"] = installedProviders + + // anonymize any unknown providers or the ones not publicly listed + id, err := store.GetProviderID(pAddr) + if err != nil { + continue + } + addr := fmt.Sprintf("unlisted/%s", id) + installedProviders[addr] = "" } + properties["installedProviders"] = installedProviders + } - telemetrySender.SendEvent(ctx, "moduleData", properties) + if !hasChanged { + return nil, false } + + modId, err := store.GetModuleID(newMod.Path) + if err != nil { + return nil, false + } + properties["moduleId"] = modId + + return properties, true } func updateDiagnostics(ctx context.Context, notifier *diagnostics.Notifier) state.ModuleChangeHook { diff --git a/internal/state/installed_providers.go b/internal/state/installed_providers.go new file mode 100644 index 000000000..eb136d147 --- /dev/null +++ b/internal/state/installed_providers.go @@ -0,0 +1,26 @@ +package state + +import ( + "github.com/hashicorp/go-version" + tfaddr "github.com/hashicorp/terraform-registry-address" +) + +type InstalledProviders map[tfaddr.Provider]*version.Version + +func (ip InstalledProviders) Equals(p InstalledProviders) bool { + if len(ip) != len(p) { + return false + } + + for pAddr, ver := range ip { + c, ok := p[pAddr] + if !ok { + return false + } + if !ver.Equal(c) { + return false + } + } + + return true +} diff --git a/internal/state/installed_providers_test.go b/internal/state/installed_providers_test.go new file mode 100644 index 000000000..ecd18c230 --- /dev/null +++ b/internal/state/installed_providers_test.go @@ -0,0 +1,80 @@ +package state + +import ( + "fmt" + "testing" + + "github.com/hashicorp/go-version" + tfaddr "github.com/hashicorp/terraform-registry-address" +) + +func TestInstalledProviders(t *testing.T) { + testCases := []struct { + first, second InstalledProviders + expectEqual bool + }{ + { + InstalledProviders{}, + InstalledProviders{}, + true, + }, + { + InstalledProviders{ + tfaddr.NewBuiltInProvider("terraform"): version.Must(version.NewVersion("1.0")), + }, + InstalledProviders{ + tfaddr.NewBuiltInProvider("terraform"): version.Must(version.NewVersion("1.0")), + }, + true, + }, + { + InstalledProviders{ + tfaddr.NewDefaultProvider("foo"): version.Must(version.NewVersion("1.0")), + }, + InstalledProviders{ + tfaddr.NewDefaultProvider("bar"): version.Must(version.NewVersion("1.0")), + }, + false, + }, + { + InstalledProviders{ + tfaddr.NewDefaultProvider("foo"): version.Must(version.NewVersion("1.0")), + }, + InstalledProviders{ + tfaddr.NewDefaultProvider("foo"): version.Must(version.NewVersion("1.1")), + }, + false, + }, + { + InstalledProviders{ + tfaddr.NewDefaultProvider("foo"): version.Must(version.NewVersion("1.0")), + tfaddr.NewDefaultProvider("bar"): version.Must(version.NewVersion("1.0")), + }, + InstalledProviders{ + tfaddr.NewDefaultProvider("foo"): version.Must(version.NewVersion("1.0")), + }, + false, + }, + { + InstalledProviders{ + tfaddr.NewDefaultProvider("foo"): version.Must(version.NewVersion("1.0")), + }, + InstalledProviders{ + tfaddr.NewDefaultProvider("foo"): version.Must(version.NewVersion("1.0")), + tfaddr.NewDefaultProvider("bar"): version.Must(version.NewVersion("1.0")), + }, + false, + }, + } + for i, tc := range testCases { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + equals := tc.first.Equals(tc.second) + if tc.expectEqual != equals { + if tc.expectEqual { + t.Fatalf("expected requirements to be equal\nfirst: %#v\nsecond: %#v", tc.first, tc.second) + } + t.Fatalf("expected requirements to mismatch\nfirst: %#v\nsecond: %#v", tc.first, tc.second) + } + }) + } +} diff --git a/internal/state/module.go b/internal/state/module.go index c10fcea5b..f6a9d38cf 100644 --- a/internal/state/module.go +++ b/internal/state/module.go @@ -19,7 +19,7 @@ type ModuleMetadata struct { CoreRequirements version.Constraints Backend *tfmod.Backend ProviderReferences map[tfmod.ProviderRef]tfaddr.Provider - ProviderRequirements map[tfaddr.Provider]version.Constraints + ProviderRequirements tfmod.ProviderRequirements Variables map[string]tfmod.Variable Outputs map[string]tfmod.Output } @@ -45,7 +45,7 @@ func (mm ModuleMetadata) Copy() ModuleMetadata { } if mm.ProviderRequirements != nil { - newMm.ProviderRequirements = make(map[tfaddr.Provider]version.Constraints, len(mm.ProviderRequirements)) + newMm.ProviderRequirements = make(tfmod.ProviderRequirements, len(mm.ProviderRequirements)) for provider, vc := range mm.ProviderRequirements { // version.Constraints is never mutated in this context newMm.ProviderRequirements[provider] = vc @@ -80,7 +80,7 @@ type Module struct { TerraformVersionErr error TerraformVersionState op.OpState - InstalledProviders map[tfaddr.Provider]*version.Version + InstalledProviders InstalledProviders ProviderSchemaErr error ProviderSchemaState op.OpState @@ -146,7 +146,7 @@ func (m *Module) Copy() *Module { } if m.InstalledProviders != nil { - newMod.InstalledProviders = make(map[tfaddr.Provider]*version.Version, 0) + newMod.InstalledProviders = make(InstalledProviders, 0) for addr, pv := range m.InstalledProviders { // version.Version is practically immutable once parsed newMod.InstalledProviders[addr] = pv