Skip to content

Commit

Permalink
Merge cockroachdb#131249
Browse files Browse the repository at this point in the history 
131249: diagnostics: send along license information on diag report r=angles-n-daemons a=angles-n-daemons

diagnostics: send along license information on diag report

To support the license change in CRDB, we'll need to begin sending license information along with diagnostics reports. This change accomplishes this by making the license getter in `license_check.go` public, and attatching its OrganizationID, LicenseID, LicenseExpiry, and Environment to the outgoing payload.

This PR also moves the `LicenseTestingKnobs` from being part of the `server.TestingKnobs` to the `base.TestingKnobs` to avoid an import cycle in the utilccl tests.

Fixes: CRDB-41230
Epic: CRDB-40209

Release note: none

Co-authored-by: angles-n-daemons <[email protected]>
  • Loading branch information
@angles-n-daemons
craig[bot] and angles-n-daemons committed Oct 4, 2024
2 parents a1d413b + 551c045 commit 7891163
Show file tree
Hide file tree
Showing 14 changed files with 180 additions and 59 deletions.
1 change: 1 addition & 0 deletions pkg/base/testing_knobs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,5 @@ type TestingKnobs struct {
TableStatsKnobs ModuleTestingKnobs
Insights ModuleTestingKnobs
TableMetadata ModuleTestingKnobs
LicenseTestingKnobs ModuleTestingKnobs
}
14 changes: 7 additions & 7 deletions pkg/ccl/utilccl/license_check.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ var GetLicenseTTL = func(
st *cluster.Settings,
ts timeutil.TimeSource,
) int64 {
license, err := getLicense(st)
license, err := GetLicense(st)
if err != nil {
log.Errorf(ctx, "unable to find license: %v", err)
return 0
Expand All @@ -209,7 +209,7 @@ func checkEnterpriseEnabledAt(
if atomic.LoadInt32(&enterpriseStatus) == enterpriseEnabled {
return nil
}
license, err := getLicense(st)
license, err := GetLicense(st)
if err != nil {
return err
}
Expand Down Expand Up @@ -240,10 +240,10 @@ func checkEnterpriseEnabledAt(
return nil
}

// getLicense fetches the license from the given settings, using Settings.Cache
// GetLicense fetches the license from the given settings, using Settings.Cache
// to cache the decoded license (if any). The returned license must not be
// modified by the caller.
func getLicense(st *cluster.Settings) (*licenseccl.License, error) {
func GetLicense(st *cluster.Settings) (*licenseccl.License, error) {
str := enterpriseLicense.Get(&st.SV)
if str == "" {
return nil, nil
Expand All @@ -263,7 +263,7 @@ func getLicense(st *cluster.Settings) (*licenseccl.License, error) {

// GetLicenseType returns the license type.
func GetLicenseType(st *cluster.Settings) (string, error) {
license, err := getLicense(st)
license, err := GetLicense(st)
if err != nil {
return "", err
} else if license == nil {
Expand All @@ -274,7 +274,7 @@ func GetLicenseType(st *cluster.Settings) (string, error) {

// GetLicenseEnvironment returns the license environment.
func GetLicenseEnvironment(st *cluster.Settings) (string, error) {
license, err := getLicense(st)
license, err := GetLicense(st)
if err != nil {
return "", err
} else if license == nil {
Expand Down Expand Up @@ -354,7 +354,7 @@ func RegisterCallbackOnLicenseChange(
// The isChange parameter indicates whether the license is actually being updated,
// as opposed to merely refreshing the current license.
refreshFunc := func(ctx context.Context, isChange bool) {
lic, err := getLicense(st)
lic, err := GetLicense(st)
if err != nil {
log.Errorf(ctx, "unable to refresh license enforcer for license change: %v", err)
return
Expand Down
11 changes: 4 additions & 7 deletions pkg/ccl/utilccl/license_check_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/ccl/utilccl/licenseccl"
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/server/license"
"github.com/cockroachdb/cockroach/pkg/settings"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
Expand Down Expand Up @@ -386,12 +385,10 @@ func TestRefreshLicenseEnforcerOnLicenseChange(t *testing.T) {
// We are changing a cluster setting that can only be done at the system tenant.
DefaultTestTenant: base.TestIsSpecificToStorageLayerAndNeedsASystemTenant,
Knobs: base.TestingKnobs{
Server: &server.TestingKnobs{
LicenseTestingKnobs: license.TestingKnobs{
Enable: true,
SkipDisable: true,
OverrideStartTime: &ts1,
},
LicenseTestingKnobs: &license.TestingKnobs{
Enable: true,
SkipDisable: true,
OverrideStartTime: &ts1,
},
},
})
Expand Down
9 changes: 3 additions & 6 deletions pkg/server/application_api/telemetry_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/ccl/utilccl/licenseccl"
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/server/apiconstants"
"github.com/cockroachdb/cockroach/pkg/server/diagnostics"
"github.com/cockroachdb/cockroach/pkg/server/diagnostics/diagnosticspb"
Expand Down Expand Up @@ -137,11 +136,9 @@ func TestThrottlingMetadata(t *testing.T) {
s := serverutils.StartCluster(t, 3, base.TestClusterArgs{
ServerArgs: base.TestServerArgs{
Knobs: base.TestingKnobs{
Server: &server.TestingKnobs{
LicenseTestingKnobs: license.TestingKnobs{
Enable: true,
OverrideStartTime: &testtime,
},
LicenseTestingKnobs: &license.TestingKnobs{
Enable: true,
OverrideStartTime: &testtime,
},
},
},
Expand Down
5 changes: 5 additions & 0 deletions pkg/server/diagnostics/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ go_library(
deps = [
"//pkg/base",
"//pkg/build",
"//pkg/ccl/utilccl",
"//pkg/ccl/utilccl/licenseccl",
"//pkg/config/zonepb",
"//pkg/keys",
"//pkg/kv",
Expand Down Expand Up @@ -61,14 +63,17 @@ go_test(
deps = [
"//pkg/base",
"//pkg/build",
"//pkg/ccl/utilccl/licenseccl",
"//pkg/security/securityassets",
"//pkg/security/securitytest",
"//pkg/server",
"//pkg/server/diagnostics/diagnosticspb",
"//pkg/testutils/diagutils",
"//pkg/testutils/serverutils",
"//pkg/util/cloudinfo",
"//pkg/util/leaktest",
"//pkg/util/log",
"//pkg/util/uuid",
"@com_github_mitchellh_reflectwalk//:reflectwalk",
"@com_github_stretchr_testify//require",
],
Expand Down
34 changes: 33 additions & 1 deletion pkg/server/diagnostics/diagnostics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ import (
"strconv"
"time"

"github.com/cockroachdb/cockroach/pkg/ccl/utilccl/licenseccl"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/server/diagnostics/diagnosticspb"
"github.com/cockroachdb/cockroach/pkg/util/cloudinfo"
"github.com/cockroachdb/cockroach/pkg/util/envutil"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
"github.com/cockroachdb/cockroach/pkg/util/system"
"github.com/cockroachdb/cockroach/pkg/util/uuid"
Expand Down Expand Up @@ -71,6 +73,7 @@ type ClusterInfo struct {
TenantID roachpb.TenantID
IsInsecure bool
IsInternal bool
License *licenseccl.License
}

// addInfoToURL sets query parameters on the URL used to report diagnostics. If
Expand All @@ -94,10 +97,33 @@ func addInfoToURL(
q.Set("nodeid", strconv.Itoa(int(nodeID)))
}

environment := ""
licenseExpiry := ""
organizationID := ""
licenseID := ""
if clusterInfo.License != nil {
license := clusterInfo.License
environment = license.Environment.String()
if license.OrganizationId != nil {
organizationUUID, err := uuid.FromBytes(license.OrganizationId)
if err != nil {
log.Infof(context.Background(), "unexpected error parsing organization id from license %s", err)
}
organizationID = organizationUUID.String()
}
if license.LicenseId != nil {
licenseExpiry = strconv.Itoa(int(license.ValidUntilUnixSec))
licenseUUID, err := uuid.FromBytes(license.LicenseId)
if err != nil {
log.Infof(context.Background(), "unexpected error parsing organization id from license %s", err)
}
licenseID = licenseUUID.String()
}
}

b := env.Build
q.Set("sqlid", strconv.Itoa(int(sqlInfo.SQLInstanceID)))
q.Set("uptime", strconv.Itoa(int(sqlInfo.Uptime)))
q.Set("licensetype", env.LicenseType)
q.Set("version", b.Tag)
q.Set("platform", b.Platform)
q.Set("uuid", clusterInfo.StorageClusterID.String())
Expand All @@ -107,6 +133,12 @@ func addInfoToURL(
q.Set("internal", strconv.FormatBool(clusterInfo.IsInternal))
q.Set("buildchannel", b.Channel)
q.Set("envchannel", b.EnvChannel)
// license type must come from the environment because it uses the base package.
q.Set("licensetype", env.LicenseType)
q.Set("organization_id", organizationID)
q.Set("license_id", licenseID)
q.Set("license_expiry_seconds", licenseExpiry)
q.Set("environment", environment)
result.RawQuery = q.Encode()
return &result
}
Expand Down
19 changes: 17 additions & 2 deletions pkg/server/diagnostics/reporter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import (

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/build"
"github.com/cockroachdb/cockroach/pkg/ccl/utilccl"
"github.com/cockroachdb/cockroach/pkg/ccl/utilccl/licenseccl"
"github.com/cockroachdb/cockroach/pkg/config/zonepb"
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/kv"
Expand Down Expand Up @@ -142,7 +144,13 @@ func (r *Reporter) ReportDiagnostics(ctx context.Context) {

report := r.CreateReport(ctx, telemetry.ResetCounts)

url := r.buildReportingURL(report)
license, err := utilccl.GetLicense(r.Settings)
if err != nil {
if log.V(2) {
log.Warningf(ctx, "failed to retrieve license while reporting diagnostics: %v", err)
}
}
url := r.buildReportingURL(report, license)
if url == nil {
return
}
Expand Down Expand Up @@ -377,13 +385,20 @@ func (r *Reporter) collectSchemaInfo(ctx context.Context) ([]descpb.TableDescrip

// buildReportingURL creates a URL to report diagnostics.
// If an empty updates URL is set (via empty environment variable), returns nil.
func (r *Reporter) buildReportingURL(report *diagnosticspb.DiagnosticReport) *url.URL {
func (r *Reporter) buildReportingURL(
report *diagnosticspb.DiagnosticReport, license *licenseccl.License,
) *url.URL {
if license == nil {
license = &licenseccl.License{}
}

clusterInfo := ClusterInfo{
StorageClusterID: r.StorageClusterID(),
LogicalClusterID: r.LogicalClusterID(),
TenantID: r.TenantID,
IsInsecure: r.Config.Insecure,
IsInternal: sql.ClusterIsInternal(&r.Settings.SV),
License: license,
}

url := reportingURL
Expand Down
81 changes: 81 additions & 0 deletions pkg/server/diagnostics/reporter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,18 @@
package diagnostics

import (
"context"
"fmt"
"testing"

"github.com/cockroachdb/cockroach/pkg/base"
build "github.com/cockroachdb/cockroach/pkg/build"
"github.com/cockroachdb/cockroach/pkg/ccl/utilccl/licenseccl"
"github.com/cockroachdb/cockroach/pkg/server/diagnostics/diagnosticspb"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/uuid"
"github.com/mitchellh/reflectwalk"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -42,3 +50,76 @@ func TestStringRedactor_Primitive(t *testing.T) {
require.Equal(t, "_", *foo.B)
require.Equal(t, "string 3", foo.C["3"])
}

func TestBuildReportingURL(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
srv := serverutils.StartServerOnly(t, base.TestServerArgs{})
defer srv.Stopper().Stop(context.Background())

report := &diagnosticspb.DiagnosticReport{
Env: diagnosticspb.Environment{
LicenseType: "Enterprise",
Build: build.Info{
Tag: "tag",
Platform: "platform",
Channel: "buildchannel",
EnvChannel: "envchannel",
},
},
Node: diagnosticspb.NodeInfo{
NodeID: 1,
},
SQL: diagnosticspb.SQLInstanceInfo{
SQLInstanceID: 2,
Uptime: 3,
},
}
licenseID, err := uuid.FromString("abc362b1-4f67-4bc0-b7dd-5628e49d2cba")
require.NoError(t, err)
organizationID, err := uuid.FromString("123362b1-4f67-4bc0-b7dd-5628e49d2321")
require.NoError(t, err)
license := &licenseccl.License{
ValidUntilUnixSec: 4,
Type: licenseccl.License_Enterprise,
Environment: 1,
LicenseId: licenseID.GetBytes(),
OrganizationId: organizationID.GetBytes(),
}
r := srv.DiagnosticsReporter().(*Reporter)
url := r.buildReportingURL(report, license)
logicalClusterUUID := r.LogicalClusterID()
storageClusterUUID := r.StorageClusterID()
require.Equal(t, fmt.Sprintf(`https://register.cockroachdb.com/api/clusters/report?buildchannel=buildchannel&envchannel=envchannel&environment=production&insecure=false&internal=false&license_expiry_seconds=4&license_id=abc362b1-4f67-4bc0-b7dd-5628e49d2cba&licensetype=Enterprise&logical_uuid=%s&nodeid=1&organization_id=123362b1-4f67-4bc0-b7dd-5628e49d2321&platform=platform&sqlid=2&tenantid=%s&uptime=3&uuid=%s&version=tag`, logicalClusterUUID, r.TenantID.String(), storageClusterUUID), url.String())
}

func TestBuildReportingURLNoLicense(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
srv := serverutils.StartServerOnly(t, base.TestServerArgs{})
defer srv.Stopper().Stop(context.Background())

report := &diagnosticspb.DiagnosticReport{
Env: diagnosticspb.Environment{
LicenseType: "OSS",
Build: build.Info{
Tag: "tag",
Platform: "platform",
Channel: "buildchannel",
EnvChannel: "envchannel",
},
},
Node: diagnosticspb.NodeInfo{
NodeID: 1,
},
SQL: diagnosticspb.SQLInstanceInfo{
SQLInstanceID: 2,
Uptime: 3,
},
}
r := srv.DiagnosticsReporter().(*Reporter)
url := r.buildReportingURL(report, nil)
logicalClusterUUID := r.LogicalClusterID()
storageClusterUUID := r.StorageClusterID()
require.Equal(t, fmt.Sprintf(`https://register.cockroachdb.com/api/clusters/report?buildchannel=buildchannel&envchannel=envchannel&environment=&insecure=false&internal=false&license_expiry_seconds=&license_id=&licensetype=OSS&logical_uuid=%s&nodeid=1&organization_id=&platform=platform&sqlid=2&tenantid=%s&uptime=3&uuid=%s&version=tag`, logicalClusterUUID, r.TenantID.String(), storageClusterUUID), url.String())
}
3 changes: 3 additions & 0 deletions pkg/server/license/enforcer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ type TestingKnobs struct {
OverwriteClusterInitGracePeriodTS bool
}

// ModuleTestingKnobs is part of the base.ModuleTestingKnobs interface.
func (*TestingKnobs) ModuleTestingKnobs() {}

// TelemetryStatusReporter is the interface we use to find the last ping
// time for telemetry reporting.
type TelemetryStatusReporter interface {
Expand Down
Loading

0 comments on commit 7891163

Please sign in to comment.