Repository navigation
fix(dedup): bind the control socket before the app starts - #189
Merged
Merged
Conversation
The agent bound /tmp/coverage_control.sock on its background thread, and nothing ordered that bind before the app's first answer to a health check. Keploy fires tests as soon as the app answers, so when the bind came late the first tests' START and END reached no one and their coverage was lost: a run reported 153 of 200 tests covered. The END that met the socket just after it came up logged "Ignoring mismatched END command". start() now binds on the calling thread, which is premain for -javaagent, before the app's main runs, and hands the bound socket to the accept thread. A bind failure still leaves the app running without dedup: start() logs it and returns false, and nothing escapes premain. An END with no START for that test publishes nothing, yet was answered ACK, so Keploy took it as delivered and waited for a record that never came. The unix control socket now answers IGNORED. A Keploy that does not know the reply reads any answer as taken, as it did before. Signed-off-by: slayerjain <shubhamkjain@outlook.com>
The smoke harness waited for the control socket file before its first command. That wait is the race Keploy loses: it fires tests as soon as the app answers, with no such wait, so the smoke could not see a late bind. START is now main's first act and nothing waits for the socket, so an agent that binds it after premain fails the smoke with the jar that ships. The smoke fails this way on JDK 8, 17 and 21 with the bind put back on the accept thread. The verify job built with -DskipTests, so the module's unit tests ran in no pipeline. It now runs them on each JDK. Signed-off-by: slayerjain <shubhamkjain@outlook.com>
On the TCP transport an END with no START for it was answered with a bare ACK and no COV, so the collector could not tell the test had gone unmeasured. Only the app's own log said so, in a pod the collector never reads. The unix socket already answers IGNORED for the same END. The SDK now sends "IGNORED <id>" ahead of the ACK. The ACK stays because it is what ends the collector's wait: a collector that does not know the frame skips it and goes on as before, where IGNORED in place of ACK would have cost it a full ACK timeout on each such test. Signed-off-by: slayerjain <shubhamkjain@outlook.com>
The agent binds its control socket in premain, so junixsocket's native library now loads there, before the app's main. Whatever that throws, premain must not stop the app: start() catches Throwable and runs the app without dedup. Nothing tested that guard; narrowed to IOException, it passed the unit tests and the smoke while a JVM that cannot load the library aborted before main. The smoke now runs an app with junixsocket's override properties pointing at a library that does not exist, and checks that main ran and that the agent said the control socket is unavailable, so the case cannot pass with the library loaded. Signed-off-by: slayerjain <shubhamkjain@outlook.com>
Keploy's agent reads an answer starting "IGNORED", and k8s-proxy's collector a frame starting "IGNORED ", as a command the SDK did not take. The rows compared the reply with the SDK's own constant, so a change to the word passed them while both readers would take every ignored END as taken. They now compare it with the literal. Signed-off-by: slayerjain <shubhamkjain@outlook.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
Java dedup lost the coverage of a run's first tests, seen as
Ignoring mismatched END command. expected=, actual=test-set-0/test-47in the Java dedup lanes (once in about 470 runs).Describe the changes you've made
The bug.
premainstarted a daemon thread and the socket was bound on that thread. Keploy fires tests as soon as the app answers its health check, and nothing ordered the bind before that. When the bind was late, each START and END met a refused dial, and the first END after the bind arrived with no START, so the SDK logged the mismatch and still answered ACK. Those tests had no coverage, and the report came out short: 153 of 200 on the run that failed.The fix.
start(), on the caller's thread, not on the accept thread. For-javaagentthat ispremain, which runs before the app'smain, so the socket listens before any app code can answer. ForKeployMiddlewareit is the filter'sinit, which the container runs before the filter's first request. An agent attached to a running app binds as it attaches.premain.IGNORED, not ACK: nothing was published for that test, and ACK said it had been.IGNORED <id>and thenACK. ACK stays because it ends the collector's wait.Compatibility. A Keploy agent that does not know
IGNOREDreads it as an answer, as it reads ACK. A k8s collector that does not know the frame skips it and takes the ACK.CI. The smoke test waited for the socket file before its first command, which is the wait Keploy does not do, so it could not see a late bind. START is now main's first act. A new smoke case runs an app with junixsocket's native library made unloadable and checks that main runs and that the agent says the socket is unavailable. The verify job built with
-DskipTests, so the unit tests ran in no pipeline; they now run on each JDK.Tested.
ControlSocketTest(socket bound whenstart()returns, stale file replaced, bind failure reported,IGNOREDon the unix socket) andCoverageTcpClientTest(IGNORED <id>thenACK, in that order, naming the test). Both compare the reply with the literal word: Keploy and k8s-proxy read that word, so it is a contract.IOException: that JVM aborts inpremainbefore main runs.premain: 200 covered, no mismatch, no stall.Type of change