Skip to content

Conversation

premrajvs
Copy link
Contributor

I added a function that gets invoked when the MCP Server starts for the first time to test the connection with Dynatrace Tenant. I also added a tool with the ability to test connection.
I tested the code with prettier function as well.
I tested the changes by starting the MCP Server with incorrect credentials and it returned error. I also tested the changes by deploying the MCP Server in a server that cannot access Dynatrace tenant and it returned error.
In actual enterprise environment, these changes will help users launch MCP Servers and validate the connectivity with tenant.

Copy link
Collaborator

@MrManny MrManny left a comment

Choose a reason for hiding this comment

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

I am wondering if we should also incorporate some aspects we've seen in real-life, specifically #33. It would mean test-querying Grail though, which may incur cost. If something like that is added, maybe we should formulate the instruction in a manner that will only call the tool if the user specifically asks for it?
WDYT?

@premrajvs
Copy link
Contributor Author

That is an interesting thought and I agree it will help the users. I will take that as an action item for a separate feature branch. However, this PR is strictly for one reason. To help users confirm - Is the MCP Server connecting to Dynatrace tenant ? because in Enterprise environments teams deploy MCP servers on a private instance and may have to allow the traffic via firewall to the tenant. This code change will help users confirm if their MCP server is connected to their respective tenant.

@MrManny
Copy link
Collaborator

MrManny commented Jul 30, 2025

Fair :)

Copy link
Collaborator

@MrManny MrManny left a comment

Choose a reason for hiding this comment

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

LGTM! 👍🚀

@pvaradharajsingaravelu-fi

@christian-kreuzberger-dtx Can you please merge the changes to main branch

@christian-kreuzberger-dtx
Copy link
Collaborator

Please hang tight, I'll verify those changes today and get them merged. Thanks for your patience!

Copy link
Collaborator

@christian-kreuzberger-dtx christian-kreuzberger-dtx left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution. I left some remarks and questions.

… env tool subscription 2) added handleClientRequestError utility function to handle errors 3) added a rety logic to testconnection and avoid endless loop
@premrajvs
Copy link
Contributor Author

@christian-kreuzberger-dtx Thank you for taking the time to share your feedback. I made the recommended changes. Please review and do the needful.

Copy link
Collaborator

@christian-kreuzberger-dtx christian-kreuzberger-dtx left a comment

Choose a reason for hiding this comment

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

Thank you, LGTM!

@christian-kreuzberger-dtx christian-kreuzberger-dtx merged commit 32d7a2d into dynatrace-oss:main Aug 18, 2025
4 checks passed
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.

4 participants