8000 AAP-43201 netceptor logging by arrestle · Pull Request #1300 · ansible/receptor · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

AAP-43201 netceptor logging #1300

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 8000 “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

Jump to bottom
Merged
merged 6 commits into from
May 8, 2025
Merged

AAP-43201 netceptor logging #1300

merged 6 commits into from
May 8, 2025

Conversation

arrestle
Copy link
Contributor
@arrestle arrestle commented Apr 11, 2025 •
edited
Loading

This PR is ready to review.

While it doesn't implement structured logging, it's organized in such a way as to make that easier to add in AAP-44416: Implement dynamic log level changes.

Note that additional diagnostics were added to identify existing connections with initial handshake errors, but does not remove them as that is a bug that is out of scope. See PR#1303 for details.

Note that several iterations later, we've decided to leave the logging in netceptor.go the way it is, just adding a few more log diagnostics and messages. The ability to add suffixes was added the logger with the addition of AddSuffix and UpdateSuffix to the logger itself. All future logs will contain what is in the log suffix.

Example output. See at 15:48:21 I stopped hop and restarted it a few seconds later.

INFO 2025/05/07 15:48:17 Known Connections: {"node_id":"execution","remote_id":"hop"}
INFO 2025/05/07 15:48:17    execution:  {"node_id":"execution","remote_id":"hop"}
INFO 2025/05/07 15:48:17    hop: control(1.00)  {"node_id":"execution","remote_id":"hop"}
INFO 2025/05/07 15:48:17    control: hop(1.00)  {"node_id":"execution","remote_id":"hop"}
INFO 2025/05/07 15:48:17 Routing Table: {"node_id":"execution","remote_id":"hop"}
WARNING 2025/05/07 15:48:21 Backend connection failed (will retry): dial tcp [::1]:2224: connect: connection refused {"remote_id":"hop","node_id":"execution"}
INFO 2025/05/07 15:48:29 Connection established with hop {"node_id":"execution","remote_id":"hop"}
INFO 2025/05/07 15:48:29 Known Connections: {"node_id":"execution","remote_id":"hop"}
INFO 2025/05/07 15:48:29    execution: hop(1.00)  {"node_id":"execution","remote_id":"hop"}
INFO 2025/05/07 15:48:29    hop: control(1.00) execution(1.00)  {"node_id":"execution","remote_id":"hop"}
INFO 2025/05/07 15:48:29    control: hop(1.00)  {"node_id":"execution","remote_id":"hop"}

@arrestle arrestle requested review from AaronH88 and lranjbar April 11, 2025 14:57
@arrestle arrestle marked this pull request as draft April 11, 2025 14:57
@codecov Codecov
Copy link
codecov bot commented Apr 11, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 82.81250% with 11 lines in your changes missing coverage. Please review.

Project coverage is 44.54%. Comparing base (06eef5e) to head (d498326).
Report is 1 commits behind head on devel.

Files with missing lines Patch % Lines
pkg/netceptor/netceptor.go 60.00% 4 Missing and 2 partials ⚠️
pkg/logger/logger.go 93.18% 2 Missing and 1 partial ⚠️
pkg/netceptor/external_backend.go 0.00% 2 Missing ⚠️ 
@@            Coverage Diff             @@
##            devel    #1300      +/-   ##
==========================================
+ Coverage   43.72%   44.54%   +0.82%     
==========================================
  Files          57       60       +3     
  Lines        9782    10423     +641     
==========================================
+ Hits         4277     4643     +366     
- Misses       5193     5465     +272     
- Partials      312      315       +3
Files with missing lines Coverage Δ
pkg/backends/tcp.go 27.87% <100.00%> (+0.97%) ⬆️
pkg/netceptor/external_backend.go 60.19% <0.00%> (+1.77%) ⬆️
pkg/logger/logger.go 61.50% <93.18%> (+17.24%) ⬆️
pkg/netceptor/netceptor.go 51.58% <60.00%> (+0.87%) ⬆️

... and 5 files with indirect coverage changes

Components Coverage Δ
Go 44.26% <82.81%> (+0.54%) ⬆️
Receptorctl 49.31% <ø> (∅)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

matoval
matoval reviewed Apr 17, 2025
View reviewed changes
@arrestle arrestle marked this pull request as ready for review April 17, 2025 21:20
@arrestle arrestle requested a review from matoval April 17, 2025 21:20
@arrestle arrestle force-pushed the aap-43085-netceptor-log branch 2 times, most recently from c032714 to 7385357 Compare April 29, 2025 21:51
PabloHiro
PabloHiro reviewed Apr 30, 2025
View reviewed changes
@arrestle arrestle requested a review from PabloHiro April 30, 2025 15:06
matoval
matoval reviewed Apr 30, 2025
View reviewed changes
@arrestle arrestle requested a review from matoval April 30, 2025 20:58
@arrestle arrestle enabled auto-merge (squash) April 30, 2025 21:02
@lranjbar
Copy link
Contributor

We have some unit tests failing perhaps this needs a rebase?

AaronH88
AaronH88 reviewed May 1, 2025
View reviewed changes
AaronH88
AaronH88 reviewed May 1, 2025
View reviewed changes
AaronH88
AaronH88 reviewed May 1, 2025
View reviewed changes
@arrestle arrestle requested a review from AaronH88 May 1, 2025 20:11
davemulford
davemulford reviewed May 2, 2025
View reviewed changes
Copy link
Contributor
@davemulford davemulford left a comment

Choose a reason for hiding this comment

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

Added two minor things that I've asked about inline. Overall looks good!

dleehr
dleehr reviewed May 2, 2025
View reviewed changes
Copy link
Contributor
@dleehr dleehr left a comment

Choose a reason for hiding this comment

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

Looks like this will be really helpful for us to review logs, so I'm excited to see it completed.

I did have a fair amount of feedback, let me know what you think!

dleehr
dleehr reviewed May 2, 2025
View reviewed changes
@arrestle arrestle requested review from dleehr and davemulford May 2, 2025 21:05
dleehr
dleehr reviewed May 5, 2025
View reviewed changes
Copy link
Contributor
@dleehr dleehr left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the feedback, nice improvements! I had a couple other thoughts, since I felt I could understand the changes better now 🥇. But nothing showstopping.

We'll see how the poll goes for how you decide to address the suffix type.

@arrestle arrestle requested a review from dleehr May 5, 2025 19:17
@arrestle arrestle force-pushed the aap-43085-netceptor-log branch from e3d7d8b to 2fde0db Compare May 5, 2025 19:51
@arrestle arrestle force-pushed the aap-43085-netceptor-log branch from 2fde0db to 2cb8d2a Compare May 5, 2025 19:57
dleehr
dleehr reviewed May 6, 2025
View reviewed changes
arrestle and others added 2 commits May 6, 2025 09:11
@arrestle @dleehr
Update pkg/logger/logger.go
8018527 
Co-authored-by: Dan Leehr <dleehr@users.noreply.github.com>
@arrestle
Revert "Update pkg/logger/logger.go"
ad96375 
This reverts commit 8018527.
@arrestle arrestle requested a review from dleehr May 6, 2025 19:52
dleehr
dleehr approved these changes May 7, 2025
9E88 View reviewed changes
AaronH88
AaronH88 reviewed May 7, 2025
View reviewed changes
AaronH88
AaronH88 reviewed May 7, 2025
View reviewed changes
@sonarqubecloud SonarQubeCloud
Copy link
sonarqubecloud bot commented May 7, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@arrestle arrestle requested a review from AaronH88 May 7, 2025 21:07
AaronH88
AaronH88 approved these changes May 8, 2025
View reviewed changes
Copy link
Contributor
@AaronH88 AaronH88 left a comment

Choose a reason for hiding this comment

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

lgtm

@arrestle arrestle merged commit 0d359e5 into ansible:devel May 8, 2025
24 checks passed
arrestle added a commit to arrestle/receptor that referenced this pull request Jun 6, 2025
@arrestle
AAP-43201 netceptor logging (ansible#1300)
d603999
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dleehr dleehr dleehr approved these changes

@AaronH88 AaronH88 AaronH88 approved these changes

@lranjbar lranjbar Awaiting requested review from lranjbar

@tyraziel tyraziel Awaiting requested review from tyraziel

@fincamd fincamd Awaiting requested review from fincamd

@PabloHiro PabloHiro Awaiting requested review from PabloHiro

@matoval matoval Awaiting requested review from matoval

@davemulford davemulford Awaiting requested review from davemulford

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@arrestle @lranjbar @davemulford @dleehr @AaronH88 @PabloHiro @matoval
0