Added a feature to enable glowroot in tomcat#148
Merged
ifoche merged 16 commits intodevelopmentfrom Oct 29, 2025
Merged
Conversation
Release 1.15.0
Release 0.15.1
…service. It downloads the latest version (--glowroot) or you can specify an specific zip file that you already have downloaded (--glowroot-zip), by default it will listen on port 4000, but you can change it with --glowroot-port (only if glowroot is enabled by any of the other two ways). Latest version is prioritized over an specific file
tokland
reviewed
Jul 24, 2025
Contributor
tokland
left a comment
tokland
left a comment
There was a problem hiding this comment.
Some minor in-line comments:
Contributor
|
A minor thing: when not using Glowroot, we see in the logs: "core-1 | warning [/opt/glowroot.zip]: zipfile is empty". The user may think something is wrong. Can we refactor to hide this or show a more descriptive log entry? |
cgbautista
force-pushed
the
feature/glowroot
branch
from
3e24a35 to
f0babd9
Compare
cgbautista
force-pushed
the
feature/glowroot
branch
from
f0babd9 to
e1ed7a8
Compare
domain in the /etc/resolv.conf copied onto the containers This avoids the `core` container trying to determine when the `data` container has finished its execution via `curl` to get responses from any `data.search_domain` machine that might exist.
to the original content (it didn't erase contents, but overwrote any change on those folders with the files' previous version)
that is passed as arguments
-q" did not return any container, so a "-a" parameter is added to ensure all processes (even stopped) are returned
tokland
reviewed
Oct 23, 2025
| if [ -f $GLOWROOT_ZIP ] && [ ! -d $GLOWROOT_DIR ] ; then | ||
| unzip -q $GLOWROOT_ZIP -d /opt/ || [ $? -le 1 ] | ||
| status=0 | ||
| output=$(unzip -q "$GLOWROOT_ZIP" -d /opt/ 2>&1) || status=$? |
Contributor
There was a problem hiding this comment.
[formatting] I'd say to test with vscode+some extension (ideally with some .editconfig so we all get the same)
src/d2_docker/glowroot.py
Outdated
|
|
||
| def get_glowroot_zip(command, glowroot_zip, glowroot): | ||
| logger = utils.logger | ||
| glowroot_path=None |
| def get_glowroot_zip(command, glowroot_zip, glowroot): | ||
| logger = utils.logger | ||
| glowroot_path=None | ||
| if isinstance(command, list) and command[0] == "up": |
Contributor
There was a problem hiding this comment.
keeping the spirit of immutability, I'd add an explicit else to better manage glowroot_path.
README.md
Outdated
| When starting a container, there are a few options to enable glowroot on the Tomcat process: | ||
| - Use option `--glowroot-port=PORT` to specify the APM glowroot port of the Tomcat process. | ||
| - Use option `--glowroot` to use the latest version of glowroot in the Tomcat process. | ||
| - Use option `--glowroot-zip=FILE` to specify the zip file with a version of glowroot to run in the Tomcat process. |
Contributor
There was a problem hiding this comment.
What's the default port if not specified?
README.md
Outdated
| When starting a container, there are a few options to enable glowroot on the Tomcat process: | ||
| - Use option `--glowroot-port=PORT` to specify the APM glowroot port of the Tomcat process. | ||
| - Use option `--glowroot` to use the latest version of glowroot in the Tomcat process. | ||
| - Use option `--glowroot-zip=FILE` to specify the zip file with a version of glowroot to run in the Tomcat process. |
Contributor
There was a problem hiding this comment.
It's not clear how to the options form a group. I guess --glowroot | glowroot-zip, and --glowroot-port applies to both.
Contributor
|
@cgbautista can you link this PR to its task in clickup? |
Contributor
|
@cgbautista can you link this PR (in the description) to some task in clickup? (this one? https://app.clickup.com/t/8699xm4qd) |
tokland
approved these changes
Oct 27, 2025
…topped_containers "d2-docker rm" not clearing up containers
…_apps [FIX] -k option should keep apps, dataValues, documents, etc. upon restart
[Fix] Core container might get responses from other servers named "data"
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added a feature to support an open source APM for the tomcat process: glowroot.
There are 3 new parameters to the
startfunction:--glowroot: it downloads the latest version of glowroot and deploys it under /opt/glowroot. Activates the APM in tomcat and allos incoming connections.--glowroot-zip=_FILE_: instead of allowing to dectect and download the latest version available, it can specify a zip file with any custom version of glowroot, in case the latest version wouldn't work with the tomcat version running or had any issues. It is ignored if--glowrootis specified.--glowroot-port=_PORT_: it allows to modify the default glowroot port exposed from 4000 to the desired one. Will not do anything if neither--glowrootnor--glowroot-zipare specified.When enabled, the exposed endpoint can be reached at
http://localhost:4000by default.