search

eclipse-cdt-cloud / theia-trace-extension

Eclipse Theia trace viewer extension using the Trace Server Protocol (TSP), through the tsp-typescript-client. Also the home for reusable JavaScript libraries: traceviewer-base, traceviewer-react-components
MIT License
49 stars 60 forks source link

Investigation: update experiments as part of the REST API (TSP) #608

Open bhufmann opened 2 years ago

bhufmann commented 2 years ago

Triggered by the server update proposal [1] and the TSP specification for that [2] some design thinking needs to decide the way forward. Updates in TSP, server and front-end will triggered based on the decision made for this.

Background:

The TSP specification [2] proposes a PUT command on the tsp/experiments/{expUUID} endpoint for the following use cases:

The implementation of a PUT command requires to be idempotent, which means calling it once or several times successively has the same effect. This means the UUID cannot change for the experiment.

The current Trace Compass server implementation, however, creates the UUID based on the experiment name and persists the experimented as folder resource in the server workspace. Renaming an experiment will require to change the UUID. However, this would break the idempotent requirement.

Moreover, it is not clear if adding or removing of traces should be seen as the same experiment. If yes, then UUID cannot change and a PUT command can be used. If it cannot be seen as a same experiment, then a POST command would be more appropriate.

This ticket is to discuss the way forward for updating of experiments. Based on the outcome, issue trackers and updates will be triggered in relevant components.

[1] https://git.eclipse.org/r/c/tracecompass.incubator/org.eclipse.tracecompass.incubator/+/187742 [2] https://github.com/theia-ide/trace-server-protocol/blob/34d244cc35e652681b2f003d3bd69100da6541d6/API.yaml#L216

bhufmann commented 2 years ago

PUT commands require to be idempotent, multiple requests have to have the same outcome. For a PUT to be idempotent, it is required to provide all parameters that makes up an experiment to be sent in the PUT command, e.g. name and traces.

PATCH commands don't have that requirement of having to be idempotent. They can be idempotent, but don't have to. It can be used to update part of the experiment resource.

For both PUT and PATCH, the experiment UUID must not change. The Trace Server implementation needs to be able to provide that.

Rename

For renaming of the experiment I think PATCH would be more suitable because only the to be modified name needs to be provided.

Add/Remove Traces

Adding and removing of traces in an experiment will affect the start/end time, available views and other capabilities. The front-end will need to be fully refreshed and any invalid views need to be removed. I would argue to close the experiment before doing this action. Alternative 1) This action can currently already be achieved by deleting the existing experiment and creating a new experiment with a new list of traces. Added traces will have to be uploaded first, removed traces need to be removed from the server. This will add more responsibility on the clients, while the server side implementation is simpler. Alternative 2) PATCH command on tsp/experiments/{expUUID}. This will update the experiment on the server. More responsibility and complexity is added to the server for handling that. Added traces will have to be uploaded first, while removed traces will be removed automatically by the server (if not in use otherwise)

Other updates (e.g. offsetting of traces)

Generally, other updates, e.g. change of experiment type, can be done using a PATCH command.

Offsetting traces in an experiment is useful to align the time from different traces. Use a PATCH command to provide an offset for each trace in the experiment. Providing a offset to a trace for given traceUUID will affect other experiments that include the same trace with the same UUID. To avoid that the offset of a given trace affects other experiments that contains the same trace a copy of the trace needs to be done that creates a new UUID. On the server, a symbolic link to the original trace file should be used to avoid costly copies. Alternative 1) The copy of the trace and offset configuration is done by the client. The experiment is updated with the copied trace. This will add more responsibility on the clients, while the server side implementation is simpler. Alternative 2) PATCH command on tsp/experiments/{expUUID}. The sever takes care of the copy and offsetting of the traces. More responsibility and complexity is added to the server for handling that.

Notes

The trace server implementation have to be able to handle the modification of the experiment concurrently with other clients sending requests towards the same experiment instances (same UUID).

Implementation approach

Implement each feature incrementally to be able to focus on one feature at a time (full-stack implementation). For each feature update API specification, server implementation, tsp-typescript-client/tsp-python-client and then front-end.

  1. Rename
  2. Add/Remove
  3. Time Offsetting
marco-miller commented 2 years ago

I think it would be worth it to benefit from the recently introduced ADR structure (#638),

bhufmann commented 2 years ago

I think it would be worth it to benefit from the recently introduced ADR structure (#638),

* as we formalize this well-progressing investigation further.

* As this experiments-related case could pave the way for other features with potentially similar requirements.

* This could also lead to some more `GraphQL` (mutations) related requirements.

I can add an ADR for this. Will give me a chance to try it out myself. Thanks for the suggestion.

bhufmann commented 2 years ago

https://github.com/eclipse-cdt-cloud/theia-trace-extension/pull/688 adds an ADR for renaming of an experiment and add/remove of traces in an experiment. Other updates, e.g. offsetting of traces, are not part of the ADR. I'll create a separate tracker for offsetting of traces.

bhufmann commented 10 months ago

I have another suggestion that is not reflected in the ADR as of today, yet. I think we could use custom command suffixes to define endpoints to do different behaviours, like rename experiment, add/remove traces from experiments. Use POST, PUT or PATCH depending on action. POST makes sense for rename because it moves one resource to a new one: