telemetry: Only send requests if data has changed

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 {

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
+}

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)
+			}
+		})
+	}
+}

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