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

gossip: remove gossip eventlog #117936

Merged
merged 1 commit into from
Jan 19, 2024

Conversation

petermattis
Copy link
Collaborator

Remove the gossip eventlog which was the last remaining use of AmbientContext.{Set,Finish}EventLog allowing those methods to be removed as well. Remove log.With{,No}EventLog which were not being used and remove the associated eventlog machinery from util/log.

Epic: none
Release note (general change): Remove the /debug/events endpoint which is no longer populated.

@petermattis petermattis requested review from a team as code owners January 18, 2024 18:28
@petermattis petermattis requested review from xinhaoz and jbowens and removed request for a team January 18, 2024 18:28
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@petermattis
Copy link
Collaborator Author

Release note (general change): Remove the /debug/events endpoint which is no longer populated.

This is true after #117928 merges.

Copy link
Member

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @erikgrinaker, @jbowens, @nvanbenschoten, @petermattis, and @xinhaoz)


pkg/util/log/trace.go line 43 at r1 (raw file):

// remain in the result.
func eventInternal(sp *tracing.Span, isErr bool, entry *logEntry) {
	if sp != nil {

I don't think we need this check anymore.


pkg/util/log/trace.go line 186 at r1 (raw file):

var _ = VErrEventfDepth // silence unused warning

// HasSpan returns true if the context has a span or event that should

remote "or event"

@petermattis petermattis force-pushed the pmattis/gossip-eventlog branch 3 times, most recently from 7bd83b0 to f17a96e Compare January 18, 2024 19:03
Copy link
Collaborator Author

@petermattis petermattis left a comment

Choose a reason for hiding this comment

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

TFTR!

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @erikgrinaker, @jbowens, @nvanbenschoten, @RaduBerinde, and @xinhaoz)


pkg/util/log/trace.go line 43 at r1 (raw file):

Previously, RaduBerinde wrote…

I don't think we need this check anymore.

Good point. Removed.


pkg/util/log/trace.go line 186 at r1 (raw file):

Previously, RaduBerinde wrote…

remote "or event"

Ack. I caught a number of other instances of "event log" that have been removed as well.

Remove the gossip eventlog which was the last remaining use of
`AmbientContext.{Set,Finish}EventLog` allowing those methods to be removed as
well. Remove `log.With{,No}EventLog` which were not being used and remove the
associated eventlog machinery from `util/log`.

Epic: none
Release note (general change): Remove the /debug/events endpoint which is no
longer populated.
@petermattis
Copy link
Collaborator Author

bors r+

@craig
Copy link
Contributor

craig bot commented Jan 19, 2024

This PR was included in a batch that timed out, it will be automatically retried

@craig
Copy link
Contributor

craig bot commented Jan 19, 2024

This PR was included in a batch that timed out, it will be automatically retried

@craig
Copy link
Contributor

craig bot commented Jan 19, 2024

Build succeeded:

@craig craig bot merged commit 64b6aa9 into cockroachdb:master Jan 19, 2024
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants