-
Notifications
You must be signed in to change notification settings - Fork 925
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
Add instrumentation for druid connection pool #9935
Conversation
} | ||
|
||
dependencies { | ||
library("com.alibaba:druid:0.2.6") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For simplicity, and since 1.0.0
was released in 2013 I'd recommend going ahead and baselining against 1.0
and renaming the module to druid-1.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@trask done, thank you
group.set("com.alibaba") | ||
module.set("druid") | ||
versions.set("[0.2.6,)") | ||
skip("1.0.30") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
skip("1.0.30") | |
skip("1.0.30") | |
assertInverse.set(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@trask assertInverse.set(true) will make muzzle3 check failed ,history check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think your best option is to set the version range back to versions.set("[0.2.6,)")
so you could add assertInverse.set(true)
. The same approach is used in
Line 9 in bc5398c
versions.set("[0.9.16,)") |
Lines 26 to 28 in bc5398c
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() { | |
return hasClassesNamed("javax.servlet.Servlet"); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can I write in this way
muzzle {
pass {
group.set("com.alibaba")
module.set("druid")
versions.set("[0.1.18,)")
skip("1.0.30")
assertInverse.set(true)
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AlchemyDing sure, I think you can also use versions.set("(,)")
if muzzle passes on all published versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@laurit done, thank you
Overall looks nice, the only open question for me is whether we should name this |
I think named alibaba-druid is better. |
resolve issue
connection pools mertric, like hikaricp, DBCP ,c3p0 , etc...