[DO NOT MERGE] Multienvironment update#3
Conversation
- Added ManagedAuthClientManager, used as a wrapper to handle several environments. - environment.ts updated to use new JSON array data structure. - Updated all APIs to use ManagedAuthClientManager wrapper functions. - formatResponse functions updated to handle several data sources - Minimal updates to instructions for LLM. More to do here
…ynatrace envs. - Refactored all related functions that would make use of it - Added checks in environment.ts to validate environments configuration before checking connections. - Added tool for LLM so it can query environment setting diagnostics. - `environment_alias` is now a required field. LLM will have to pass always one value to this tool param.
- Reworked environment.test.ts to touch more cases with the new multienvironment approach. - Fixed 2 out of 4 integrations test files (working on those other 2) - Fixed getEntityRelationships to return expected response structure - Fixed ManagedAuthClient to instance proxy data on creation - Removed 'dynatraceUrl' from required configuration fields. - Updated .env.template
- Auto formatting of some files - Added "dotenv/config" to jest configuration to read from .env file during integration tests
- Replaced usage of arrays in favor of Map<string, TypeInterface> for handling responses - Updated unit and integration tests to work with the new approach
- Removed unused symbols
…own. - Added flushLogger() call on shutdownHandler to avoid potentially missing log information on process.exit()
Updating README for multi-env, plus spelling/grammar/syntax/formatting
|
Can we add a few lines to the readme about reload/refresh, for when the environment config has been changed (e.g. someone edits the mcp.json file). Ivan asked in dynatrace-oss#16 for: |
|
For Ivan's github issue, can you check what behaviour he wanted if we could not connect to any of the environments? |
|
This branch says it has conflicts with |
|
The examples in docs/DEVELOPMENT.md still use |
|
(Also worth grep'ing the entire repo for |
|
A couple of tests mention |
grep -R -E "DT_API_ENDPOINT_URL|DT_DYNATRACE_URL|DT_MANAGED_API_TOKEN|DT_MANAGED_ENVIRONMENT" *Gives: |
| logger.error(`2. DT_MANAGED_API_TOKEN has required scopes: ${MANAGED_API_SCOPES.join(', ')}`); | ||
| logger.error('3. Network connectivity to the Managed cluster'); | ||
|
|
||
| process.exit(2); |
There was a problem hiding this comment.
Previous behaviour was that if we could not connect then we would quit / fail-fast. So I think it makes sense to keep that behaviour (if we can't connect to any of the environment then we quit). I'll ask Ivan for confirmation though.
There was a problem hiding this comment.
Waiting on input for make any changes related to this (if necessary)
…make clear where the data is coming from. - MINIMUM_VERSION added to ManagedAuthClientManager. Added to instructions. - Removed old references to non-existing env vars. - Small changes to LLM instructions. - README.md and DEVELOPMENT.md changes for easier reading.
It was a one-liner conflict in the README.md file. Solved. |
- Rephrased instructions so LLMs don't skip call to get_environments_info. - Added instructions for the LLM to return URLs from responses to the user - Added auxiliary method to ManagedAuthClientManager - Updated tests
… multienvironment-update
- Avoid confusion of calling SaaS a "Dynatrace enviornment" etc
2 participants
// This is for review only; do not merge here. When we are ready, we will create PR against the true upstream repo (github.com/dynatrace-oss/dynatrace-managed-mcp).
Note/tip: when comparing the files changed, add
?w=1at the end of the URL. That tells it to ignore white space. This is very useful for reviewing the code changes, especially for things like formatList methods where we have an additional loop around the code, so indents have changed without actually changing the original code. (Let's include that tip when we open the PR against the real dynatrace-oss repo)