Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

telemetry: add StartupEventDetails.Overrides #4563

Merged
merged 2 commits into from
Sep 20, 2022
Merged

telemetry: add StartupEventDetails.Overrides #4563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
613079a
telemetry: add StartupEventDetails.Overrides
cce Sep 20, 2022
17c1525
report array of Overrides
cce Sep 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions cmd/algod/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,12 +327,17 @@ func run() int {
}

currentVersion := config.GetCurrentVersion()
var overrides []telemetryspec.NameValue
for name, val := range config.GetNonDefaultConfigValues(cfg, startupConfigCheckFields) {
overrides = append(overrides, telemetryspec.NameValue{Name: name, Value: val})
}
startupDetails := telemetryspec.StartupEventDetails{
Version: currentVersion.String(),
CommitHash: currentVersion.CommitHash,
Branch: currentVersion.Branch,
Channel: currentVersion.Channel,
InstanceHash: crypto.Hash([]byte(absolutePath)).String(),
Overrides: overrides,
}

log.EventWithDetails(telemetryspec.ApplicationState, telemetryspec.StartupEvent, startupDetails)
Expand Down Expand Up @@ -369,6 +374,30 @@ func run() int {
return 0
}

var startupConfigCheckFields = []string{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider reporting all overrides so that we don't have to keep updating this in the future (or forgetting to).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to keep it very limited and pre-defined to start, because there are a lot of various ways we automatically change config values at startup time, and there are also config vars that are private or not interesting/useful to know if they were changed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having to remember later on what we do/don't care about can brittle.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not adding these add annotations to the config struct, and let GetNonDefaultConfigValues process annotations and not this map stored very separately from the config?
Or at least put in into localTemplate.go with a comment explaining what is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh a struct tag would be a good idea!

"AgreementIncomingBundlesQueueLength",
"AgreementIncomingProposalsQueueLength",
"AgreementIncomingVotesQueueLength",
"BroadcastConnectionsLimit",
"CatchupBlockValidateMode",
"ConnectionsRateLimitingCount",
"ConnectionsRateLimitingWindowSeconds",
"GossipFanout",
"IncomingConnectionsLimit",
"IncomingMessageFilterBucketCount",
"IncomingMessageFilterBucketSize",
"LedgerSynchronousMode",
"MaxAcctLookback",
"MaxConnectionsPerIP",
"OutgoingMessageFilterBucketCount",
"OutgoingMessageFilterBucketSize",
"ProposalAssemblyTime",
"ReservedFDs",
"TxPoolExponentialIncreaseFactor",
"TxPoolSize",
"VerifiedTranscationsCacheSize",
}

func resolveDataDir() string {
// Figure out what data directory to tell algod to use.
// If not specified on cmdline with '-d', look for default in environment.
Expand Down
29 changes: 29 additions & 0 deletions config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"strings"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/algorand/go-algorand/protocol"
Expand Down Expand Up @@ -551,3 +552,31 @@ func TestLocalVersionField(t *testing.T) {
expectedTag = expectedTag[:len(expectedTag)-1]
require.Equal(t, expectedTag, string(field.Tag))
}

func TestGetNonDefaultConfigValues(t *testing.T) {
partitiontest.PartitionTest(t)

cfg := GetDefaultLocal()

// set 4 non-default values
cfg.AgreementIncomingBundlesQueueLength = 2
cfg.AgreementIncomingProposalsQueueLength = 200
cfg.TxPoolSize = 30
cfg.Archival = true

// ask for 2 of them
ndmap := GetNonDefaultConfigValues(cfg, []string{"AgreementIncomingBundlesQueueLength", "TxPoolSize"})

// assert correct
expected := map[string]interface{}{
"AgreementIncomingBundlesQueueLength": uint64(2),
"TxPoolSize": int(30),
}
assert.Equal(t, expected, ndmap)

// ask for field that doesn't exist: should skip
assert.Equal(t, expected, GetNonDefaultConfigValues(cfg, []string{"Blah", "AgreementIncomingBundlesQueueLength", "TxPoolSize"}))

// check unmodified defaults
assert.Empty(t, GetNonDefaultConfigValues(GetDefaultLocal(), []string{"AgreementIncomingBundlesQueueLength", "TxPoolSize"}))
}
22 changes: 22 additions & 0 deletions config/migrate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,3 +198,25 @@ func getVersionedDefaultLocalConfig(version uint32) (local Local) {
}
return
}

// GetNonDefaultConfigValues takes a provided cfg and list of field names, and returns a map of all values in cfg
// that are not set to the default for the latest version.
func GetNonDefaultConfigValues(cfg Local, fieldNames []string) map[string]interface{} {
defCfg := GetDefaultLocal()
ret := make(map[string]interface{})

for _, fieldName := range fieldNames {
defField := reflect.ValueOf(defCfg).FieldByName(fieldName)
if !defField.IsValid() {
continue
}
cfgField := reflect.ValueOf(cfg).FieldByName(fieldName)
if !cfgField.IsValid() {
continue
}
if !reflect.DeepEqual(defField.Interface(), cfgField.Interface()) {
ret[fieldName] = cfgField.Interface()
}
}
return ret
}
7 changes: 7 additions & 0 deletions logging/telemetryspec/event.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,20 @@ type Event string
// StartupEvent event
const StartupEvent Event = "Startup"

// NameValue defines a named value, for use in an array reported to telemetry.
type NameValue struct {
Name string
Value interface{}
}

// StartupEventDetails contains details for the StartupEvent
type StartupEventDetails struct {
Version string
CommitHash string
Branch string
Channel string
InstanceHash string
Overrides []NameValue
}

// HeartbeatEvent is sent periodically to indicate node is running
Expand Down