otel.instrumentation.common.peer-service-mapping support glob #13434
Conversation
...main/java/io/opentelemetry/instrumentation/api/incubator/semconv/util/internal/GlobUtil.java
Show resolved
Hide resolved
Add comments for copied methods.
…apping-support-glob
…apping-support-glob
xiepuhuan
force-pushed
the
common-peer-service-mapping-support-glob
branch
from
fe0a054 to
1bb8548
Compare
.../java/io/opentelemetry/instrumentation/api/incubator/semconv/util/internal/GlobUtilTest.java
Show resolved
Hide resolved
| String url = "https://" + entry.getKey(); | ||
| String serviceName = entry.getValue(); | ||
|
|
||
| String host = GlobUrlParser.getHost(url); |
There was a problem hiding this comment.
can we use the regular UrlParser and just add one method to it getPortAsString()?
(basically requiring that the entry still looks like a url, just maybe not with an integer port)
There was a problem hiding this comment.
can we use the regular
UrlParserand just add one method to itgetPortAsString()?(basically requiring that the entry still looks like a url, just maybe not with an integer port)
Is the goal to let port support glob?
There was a problem hiding this comment.
do you need it? if not maybe we can simplify and only support glob on address for now
There was a problem hiding this comment.
do you need it? if not maybe we can simplify and only support glob on address for now
Yes, I don't think it's necessary.
| String url = "https://" + entry.getKey(); | ||
| String serviceName = entry.getValue(); | ||
|
|
||
| String host = GlobUrlParser.getHost(url); |
There was a problem hiding this comment.
can we use the regular
UrlParserand just add one method to itgetPortAsString()?(basically requiring that the entry still looks like a url, just maybe not with an integer port)
Is the goal to let port support glob?
| return matchers.entrySet().stream() | ||
| .filter(entry -> entry.getKey().matches(port, pathSupplier)) | ||
| .max((o1, o2) -> matcherComparator.compare(o1.getKey(), o2.getKey())) | ||
| .findFirst() | ||
| .map(Map.Entry::getValue) | ||
| .orElse(null); | ||
| } |
There was a problem hiding this comment.
The original code needs sorting to get the maximum result when there are multiple results after each filter. I think the time complexity is relatively high.
I modified the implementation to sort in the constructor. After each filter, there are multiple results. The first result found is the largest.
Related to #13111.
The host portion of the URL supports glob patterns.