Skip to content

Conversation

@kirqz23
Copy link
Contributor

@kirqz23 kirqz23 commented Oct 29, 2025

This PR introduces Beholder logs streaming test in CRE. We leverage local CRE environment with CTF observability framework. The test is suppose to validate if logs with beholder_data_type = zap_log_message are sent from chainlink nodes to observability stack (OTel collector) and Loki.

Related to: #19089

@kirqz23 kirqz23 requested review from a team as code owners October 29, 2025 09:40
@kirqz23 kirqz23 marked this pull request as draft October 29, 2025 09:40
@kirqz23 kirqz23 changed the title INFOPLAT-2875 Beholder logs streaming test INFOPLAT-2875: Beholder logs streaming test Oct 29, 2025
@kirqz23 kirqz23 marked this pull request as ready for review October 31, 2025 11:53
@kirqz23 kirqz23 marked this pull request as draft October 31, 2025 11:53
@trunk-io

This comment was marked as outdated.

@kirqz23 kirqz23 changed the title INFOPLAT-2875: Beholder logs streaming test INFOPLAT-2875: Beholder logs streaming test in CRE Oct 31, 2025
@kirqz23 kirqz23 marked this pull request as ready for review November 4, 2025 10:36
@kirqz23 kirqz23 requested review from a team as code owners November 4, 2025 11:08
@kirqz23 kirqz23 marked this pull request as draft November 4, 2025 13:18
@kirqz23 kirqz23 force-pushed the infoplat-2875-begikder-logs-test branch from 5eec63b to 793d1e6 Compare November 7, 2025 12:34
@kirqz23 kirqz23 force-pushed the infoplat-2875-begikder-logs-test branch from 81f9094 to 135d4bd Compare November 7, 2025 18:57
@kirqz23 kirqz23 requested a review from krehermann November 7, 2025 20:03
@kirqz23 kirqz23 marked this pull request as ready for review November 7, 2025 20:03
hendoxc
hendoxc previously approved these changes Nov 7, 2025
testLogger.Info().Msg("Starting Log Streaming Test")

testLogger.Info().Msg("Waiting for logs to accumulate...")
time.Sleep(60 * time.Second)
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the safe wait time, to wait for logs to accumulate from chainlink nodes to loki through beholder and OTel collector

Copy link
Collaborator

@pavel-raykov pavel-raykov Nov 10, 2025

Choose a reason for hiding this comment

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

every hard coded sleep like that 1. increases the overall testing wait time by at least one minutes, and 2. is prone to flakiness once the underlying code may exceed one minute deadline. Would you be able to switch to "require.Eventually" with periodic polling here? (the advantage is that you can introduce periodic polling with t.Deadline() and avoid hard coded deadlines)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good advice, replaced

@kirqz23 kirqz23 requested review from jmank88 and krehermann November 7, 2025 23:20
krehermann
krehermann previously approved these changes Nov 8, 2025
q.Set("limit", "10")
u.RawQuery = q.Encode()

req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, u.String(), nil)
Copy link
Collaborator

Choose a reason for hiding this comment

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

please propagate testing.T.context() here

ettec
ettec previously approved these changes Nov 10, 2025
return false
}
return beholderLogsCount > 0
}, 5*time.Minute, 5*time.Second, "Expected to find logs with beholder_data_type=zap_log_message in Loki within timeout")
Copy link
Collaborator

Choose a reason for hiding this comment

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

@cl-sonarqube-production
Copy link

@pavel-raykov pavel-raykov added this pull request to the merge queue Nov 10, 2025
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.

7 participants