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.

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

Log critical consensus values during health checks #2609

Merged
merged 2 commits into from
Jan 15, 2024

Conversation

StephenButtolph
Copy link
Contributor

Why this should be merged

Adds logging of consensus critical values when performing the health check.

How this works

When verbose logging is enabled, we'll now log critical values like outstanding polls, blocked blocks, and other critical values.

How this was tested

Ran a node with verbose logging and made sure the log showed correctly.

@StephenButtolph StephenButtolph added consensus This involves consensus monitoring This primarily focuses on logs, metrics, and/or tracing labels Jan 15, 2024
@StephenButtolph StephenButtolph added this to the v1.10.18 milestone Jan 15, 2024
@StephenButtolph StephenButtolph self-assigned this Jan 15, 2024
@@ -556,6 +556,15 @@ func (t *Transitive) HealthCheck(ctx context.Context) (interface{}, error) {
t.Ctx.Lock.Lock()
defer t.Ctx.Lock.Unlock()

t.Ctx.Log.Verbo("running health check",
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

return json.Marshal(m.keyToValue)
}

func (m *BiMap[K, V]) UnmarshalJSON(b []byte) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is the custom unmarshaller used?

Copy link
Contributor

Choose a reason for hiding this comment

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

or is this just moreso ensuring if we add marshal, we should also have unmarshal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It isn't used. I can remove it if you'd prefer not to have it. Although I felt like implementing json marshal on a normal data structure and not supporting unmarshaling was a bit weird.

@StephenButtolph StephenButtolph added this pull request to the merge queue Jan 15, 2024
Merged via the queue into dev with commit dc48be8 Jan 15, 2024
17 checks passed
@StephenButtolph StephenButtolph deleted the improve-consensus-logging branch January 15, 2024 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus This involves consensus monitoring This primarily focuses on logs, metrics, and/or tracing
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants