Skip to content

Add custom analysis related APIs #87

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.

By clicking “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

Merged

Conversation

bhufmann
Copy link
Collaborator

@bhufmann bhufmann commented Nov 11, 2024

Changes (multiple commits)

  • Support parentId and creation configuration in output descriptors
  • Add APIs to support configurable output (data provider)
    • Add configuration service per output (data provider)
      This addition enables clients to create derived data providers from
      an existing data provider.
    • Add unit tests
    • Add cli to invoke new endpoints
  • Support JSON file for load-configuration and update-configuration
    This allows users to use a file with JSON parameters.
  • Extract method for parsing/validating options.params/option.json_file

To Test

With Trace Compass Trace Server of today, the new CLI commands below can be used to configure InAndOut analysis, with LTTng Kernel traces or UST traces with function entry or exit.

  ./tsp_cli_client --list-output-configuration-sources OUTPUT_ID --uuid UUID
  ./tsp_cli_client --list-output-configuration-source TYPE_ID --output-id OUTPUT_ID --uuid UUID
  ./tsp_cli_client --create-output OUTPUT_ID --uuid UUID --json-file absolute-path-to-json-file
  ./tsp_cli_client --delete-output DERIVED_OUTPUT_ID --output-id  OUTPUT_ID --uuid UUID

where:
OUTPUT_ID = org.eclipse.tracecompass.incubator.inandout.core.analysis.inAndOutdataProviderFactory
TYPE_ID = org.eclipse.tracecompass.incubator.internal.inandout.core.config
UUID = UUID of experiment

Content of JSON file:

{
   "name": "my other InAndOut",
   "description" : "my description",
   "sourceTypeId": "org.eclipse.tracecompass.incubator.internal.inandout.core.config",
   "parameters": {
       "specifiers" : [
           {
           "label":"latency",
           "inRegex":"(\\S*)_entry",
           "outRegex":"(\\S*)_exit",
           "contextInRegex":"(\\S*)_entry",
           "contextOutRegex":"(\\S*)_exit",
           "classifier":"CPU"
           }
       ]
   }
}

Note: This PR includes changes of #86 which needs to merged before.

Signed-off-by: Bernd Hufmann [email protected]

# COMPATIBLE_PROVIDERS_KEY: obj.compatible_providers
}
result = {}
# optinal parent_id

Choose a reason for hiding this comment

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

optional

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@@ -41,6 +41,7 @@ from tsp.tsp_client import TspClient

TRACE_MISSING = "Trace UUID is missing"


Choose a reason for hiding this comment

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

extra line or intentional?

edit: Ok, It gets removed in the next commit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes

@@ -160,6 +160,18 @@ optional arguments:
--params PARAMS comma separated key value pairs (key1=val1,key2=val2)
--get-health Get the health status of the server
--get-identifier Identify important information regarding the server and the system
--list-output-configuration-sources OUTPUT_ID
Get available configuration sources for a given experiment and output
--list-output-configuration-source TYPE_ID

Choose a reason for hiding this comment

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

Is it missing OUTPUT_ID ?

Oh, I see it has to be provided as a separate argument. It's not really clear which arguments must be provided together. But that's the way it is I guess. The user will get a proper error message when missing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it's not clear at all. That's why I added the examples below to show all options needed per command. The help output (./tsp_typescript_client --help) is not good.

@@ -160,6 +160,18 @@ optional arguments:
--params PARAMS comma separated key value pairs (key1=val1,key2=val2)
--get-health Get the health status of the server
--get-identifier Identify important information regarding the server and the system
--list-output-configuration-sources OUTPUT_ID

Choose a reason for hiding this comment

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

Do these new options need to be added after line 91 also?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, they are missing.

@bhufmann bhufmann force-pushed the dp_config_updates_all_in_one branch from 0d71253 to 94de35c Compare November 12, 2024 13:54
PatrickTasse
PatrickTasse previously approved these changes Nov 12, 2024
- Add configuration service per output (data provider)
  This addition enables clients to create derived data providers from
  an existing data provider.
- Add unit tests
- Add cli to invoke new endpoints
- Update README.md for new commands

Signed-off-by: Bernd Hufmann <[email protected]>
This allows users to use a file with JSON parameters.

Signed-off-by: Bernd Hufmann <[email protected]>
@bhufmann bhufmann dismissed PatrickTasse’s stale review November 12, 2024 19:51

The merge-base changed after approval.

@bhufmann bhufmann force-pushed the dp_config_updates_all_in_one branch from 94de35c to 3f84c81 Compare November 12, 2024 19:51
@bhufmann bhufmann merged commit 10b977f into eclipse-cdt-cloud:master Nov 12, 2024
2 checks passed
@bhufmann bhufmann deleted the dp_config_updates_all_in_one branch November 12, 2024 20:13
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