Skip to content

Feature/config manager pull telemetry - #61

Open
dallinjsevy wants to merge 5 commits into
mainfrom
feature/config-manager-pull-telemetry
Open

Feature/config manager pull telemetry#61
dallinjsevy wants to merge 5 commits into
mainfrom
feature/config-manager-pull-telemetry

Conversation

@dallinjsevy

Copy link
Copy Markdown

add api and ops functions for config-manager telemetry commands

@dallinjsevy
dallinjsevy requested a review from phalestrivir July 21, 2026 21:59
@dallinjsevy
dallinjsevy force-pushed the feature/config-manager-pull-telemetry branch from d99f32e to fbb458c Compare July 21, 2026 22:07

@phalestrivir phalestrivir left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

After making your changes, make sure to run npm run build, and you will see some changes in the Help.ts file which you need to commit.

After making your changes, run npm run lint:fix, and you will see some changes that need to be committed as well.

The other thing is to add tests for TelemetryOps, since currently we don't have any tests for any of the functions. In order to have tests for this, you are going to need to also implement delete functions in the API so you can delete test data. You will definitely need this since you can only have one exporter in the tenant at a time while running the tenant, and we'll want to test both importing splunk and otlp exporters.

Comment thread src/api/cloud/TelemetryApi.ts Outdated
Comment thread src/api/cloud/TelemetryApi.ts Outdated
Comment thread src/api/cloud/TelemetryApi.ts Outdated
Comment thread src/api/cloud/TelemetryApi.ts Outdated
Comment thread src/api/cloud/TelemetryApi.ts
Comment thread src/ops/cloud/TelemetryOps.ts Outdated
Comment thread src/ops/cloud/TelemetryOps.ts Outdated
Comment thread src/lib/FrodoLib.ts
Comment thread src/api/cloud/TelemetryApi.ts Outdated
Comment thread src/api/cloud/TelemetryApi.ts Outdated
@phalestrivir

Copy link
Copy Markdown

I forgot to mention in my PR review, but we should also add a function for updateTelemetry in the OPS file. It should be simple to implement since it is essentially just a wrapper for the PUT API function similar to your readTelemetry function.

Also, add a function for deleteTelemetry that takes in an optional id and category, and have it delete everything by default unless an id or category is provided, then only delete those specific ones.

We probably won't need all these for config-manager, but we may as well add them since they are pretty simple to implement.

Comment thread src/api/cloud/TelemetryApi.ts Outdated
Comment thread src/api/cloud/TelemetryApi.ts Outdated
Comment thread src/ops/cloud/TelemetryOps.test.ts Outdated
Comment thread src/ops/cloud/TelemetryOps.test.ts
Comment thread src/ops/cloud/TelemetryOps.ts Outdated
Comment thread src/ops/cloud/TelemetryOps.ts
Comment thread src/ops/cloud/TelemetryOps.ts Outdated
Comment thread src/ops/cloud/TelemetryOps.ts Outdated
Comment thread src/ops/cloud/TelemetryOps.ts
Comment thread src/utils/ExportImportUtils.ts
@dallinjsevy
dallinjsevy force-pushed the feature/config-manager-pull-telemetry branch from 0d7692f to 93cdad9 Compare August 3, 2026 19:04

@phalestrivir phalestrivir left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A few more minor things left

Comment thread src/ops/cloud/TelemetryOps.ts Outdated
Comment thread src/ops/cloud/TelemetryOps.ts Outdated
Comment thread src/ops/cloud/TelemetryOps.ts Outdated
Comment thread src/ops/cloud/TelemetryOps.ts Outdated
Comment thread src/ops/cloud/TelemetryOps.ts Outdated
Comment thread src/ops/cloud/TelemetryOps.ts Outdated
Comment thread src/ops/cloud/TelemetryOps.ts
Comment thread src/ops/cloud/TelemetryOps.ts Outdated
Comment thread src/ops/cloud/TelemetryOps.ts Outdated

@phalestrivir phalestrivir left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Make sure to run npm run build again, when I ran it locally there were some changes that were missing from the Help.ts file.

After you do that, squash your commits, rebase with main again (since there are some new changes that I just synced to main this morning), and then make the PR against Rockcarver once you've verified the tests are still passing.

@phalestrivir phalestrivir left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actually, I think we do need one more change. We should update ConfigOps to handle exporting telemetry, since it should support it (currently we don't export telemetry in Frodo). Now that you have the export/import functions implemented, we should update ConfigOps.ts to export and import it as part of the full export/import commands.

@phalestrivir
phalestrivir force-pushed the feature/config-manager-pull-telemetry branch from 3528b9f to d0c4915 Compare August 10, 2026 17:22

@phalestrivir phalestrivir left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I updated it to do the full export/import with telemetry in the most recent commit. I rebased with main as well, so you will need to run git reset --hard origin/feature/config-manager-pull-telemetry on your local version of the branch to get the changes.

Feel free to squash the commits, but I wouldn't create the PR until the pull command is ready. We are going to need to update the pull command PR to update the config export/import tests to pass with telemetry since they currently aren't working for telemetry. Once we have the CLI PR ready against Rockcarver we can submit this one as well.

@dallinjsevy
dallinjsevy force-pushed the feature/config-manager-pull-telemetry branch from d0c4915 to 393cff3 Compare August 19, 2026 16:27
@phalestrivir
phalestrivir force-pushed the feature/config-manager-pull-telemetry branch from 393cff3 to d0c4915 Compare August 19, 2026 16:30
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