Skip to content

Conversation

scott-cotton
Copy link
Member

No description provided.

Copy link

gitar-bot bot commented Oct 10, 2025

Gitar automatically fixes CI failures and addresses comments starting with Gitar.

⚙️ Options:

  • Fix CI failures*

🔄 To revert changes, post a comment:

Gitar revert to commit <desired commit SHA>

📚 Docs

* Gitar never force pushes to your branch

@scott-cotton scott-cotton force-pushed the tw-continuous-ready-match branch from 9050be7 to 63cc7b1 Compare October 10, 2025 10:32
@daniel-de-vera daniel-de-vera self-requested a review October 10, 2025 12:30
in terminal logs and directory output meta.jsons, this will
output a json

```
{"middlewareRequestID":"c3e36c64","doneAt":"2025-10-13T17:06:32.026094+02:00"}
{"middlewareRequestID":"777bfc30","doneAt":"2025-10-13T17:06:38.413933+02:00"}
{"middlewareRequestID":"777bfc30","watchOptions":"-trunc(0)/-trunc(0)","when":"2025-10-13T15:06:38.385144025Z","normHost":"location.hotrod-devmesh.svc.cluster.local:8081","routingKey":"g9wqf90gqw9mj","destWorkload":"Deployment/hotrod-devmesh/location","requestURI":"http://sbr-dep-location-2c37fc13.hotrod-devmesh.svc:8081/locations","method":"GET","proto":"HTTP/1.1","userAgent":"curl/8.7.1"}
```

Note the order between doneAt and the initial request is not guaranteed
(I think it would be hard and likely buggy to do so...)

In request directories, the meta.jsons gets updated to include "doneAt":
@daniel-de-vera
Copy link
Contributor

@scott-cotton, here is another issue:

$ signadot sandbox apply -f - <<EOF
name: test1
spec:
  cluster: test
  virtual:
  - name: location
    workload:
      kind: Deployment
      namespace: hotrod-devmesh
      name: location
  routing:
    forwards:
      - name: test
        port: 8081
        toLocal: "localhost:18081"
EOF

$ signadot traffic watch --sandbox test1 --dir /tmp/tw1
Error: 400 Bad Request: sandboxes with spec.local or spec.routing.forwards must be created via the Signadot CLI

scott-cotton and others added 2 commits October 14, 2025 02:16
- overwrite on first apply
- fix retErrs overwrites
- fix a couple of logic issues
* adds doneAt reporting

in terminal logs and directory output meta.jsons, this will
output a json

```
{"middlewareRequestID":"c3e36c64","doneAt":"2025-10-13T17:06:32.026094+02:00"}
{"middlewareRequestID":"777bfc30","doneAt":"2025-10-13T17:06:38.413933+02:00"}
{"middlewareRequestID":"777bfc30","watchOptions":"-trunc(0)/-trunc(0)","when":"2025-10-13T15:06:38.385144025Z","normHost":"location.hotrod-devmesh.svc.cluster.local:8081","routingKey":"g9wqf90gqw9mj","destWorkload":"Deployment/hotrod-devmesh/location","requestURI":"http://sbr-dep-location-2c37fc13.hotrod-devmesh.svc:8081/locations","method":"GET","proto":"HTTP/1.1","userAgent":"curl/8.7.1"}
```

Note the order between doneAt and the initial request is not guaranteed
(I think it would be hard and likely buggy to do so...)

In request directories, the meta.jsons gets updated to include "doneAt":

* fixes to continuous readiness checking

- overwrite on first apply
- fix retErrs overwrites
- fix a couple of logic issues
@scott-cotton
Copy link
Member Author

@daniel-de-vera I merged the addition of doneAt here for workflow reasons.

It contains fixes for your comments above, in addition to @foxish 's suggestion to overwrite on first apply.

It does not yet contain adding local machine id when needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

$ signadot traffic watch --sandbox test1 --short
Waiting (up to --wait-timeout=30s) for sandbox to be ready...
✓ Sandbox status: Ready: All desired workloads are available.
msg="watching sandbox request activity" sandbox=test1 watch-options=-trunc(0)/-trunc(0) output=<none>


msg=incoming-request sandbox=test1 request.id=d4404429 request.normHost=location.hotrod-devmesh.svc.cluster.local:8081 request.dest=Deployment/hotrod-devmesh/location request.uri=http://location.hotrod-devmesh.svc:8081/locations request.method=GET request.userAgent=curl/8.5.0
msg=request-done sandbox=test1 request.id=d4404429 request.doneAt=2025-10-14T14:08:41.42059431-03:00


msg=request-done sandbox=test1 request.id=2dcf3f81 request.doneAt=2025-10-14T14:08:46.02429807-03:00
msg=incoming-request sandbox=test1 request.id=2dcf3f81 request.normHost=location.hotrod-devmesh.svc.cluster.local:8081 request.dest=Deployment/hotrod-devmesh/location request.uri=http://location.hotrod-devmesh.svc:8081/locations request.method=GET request.userAgent=curl/8.5.0


msg=request-done sandbox=test1 request.id=909417b3 request.doneAt=2025-10-14T14:08:51.290361624-03:00
msg=incoming-request sandbox=test1 request.id=909417b3 request.normHost=location.hotrod-devmesh.svc.cluster.local:8081 request.dest=Deployment/hotrod-devmesh/location request.uri=http://location.hotrod-devmesh.svc:8081/locations request.method=GET request.userAgent=curl/8.5.0

It's odd to find the request-done event in stdout before the incoming-request for the same request.id.
Do you think there could be a simple solution to this?

Copy link
Member Author

Choose a reason for hiding this comment

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

as mentioned previously, no.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have however stacked #223 to address this.

I think it technically removes the limited memory guarantee, unfortunately, but in practice it would be near impossible for that to become an issue.

I also think it solves a race that would be hard to witness, being quite unlikely but technically possible, where the updated meta.json file could have a doneAt addition attempted before the initial write. I don't think that's practically likely b/c it waits for the request/response payload files to be written. But, technically the race I believe is there without the proposed change.

scott-cotton and others added 4 commits October 14, 2025 22:12
* implementation of tw directory default

also,
- improve help
- improve signal handling

* add --clean flag

* fix sentence in help

* use -{json,yaml} for directory

* update flags after discussion

- no-instrument is hidden
- --to has become --output-dir, which only applies when !short
- --output-file may be provided with --short

* check match consistency as well.

* rename watch command to record

* add 'r' alias for record

* rename files to reflect 'record' command name

* use trafficwatch.MiddlewareName everywhere

* rename cli mw to trafficwatch from trafficwatch-client

(depends on PR in signadot repo)
Copy link
Contributor

@daniel-de-vera daniel-de-vera left a comment

Choose a reason for hiding this comment

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

LGTM

@scott-cotton scott-cotton merged commit da1c140 into main Oct 16, 2025
@scott-cotton scott-cotton deleted the tw-continuous-ready-match branch October 16, 2025 15:12
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.

2 participants