-
Notifications
You must be signed in to change notification settings - Fork 29
Mdc aspect implementation #488
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
base: master
Are you sure you want to change the base?
Mdc aspect implementation #488
Conversation
5fad817 to
4d822b8
Compare
logging/logging-common/src/main/java/ru/tinkoff/kora/logging/common/annotation/Mdc.java
Show resolved
Hide resolved
logging/declarative-logging/declarative-logging-annotation-processor/build.gradle
Outdated
Show resolved
Hide resolved
3d9a90d to
fd91803
Compare
| final Set<String> keys = new HashSet<>(); | ||
| for (AnnotationMirror annotation : methodAnnotations) { | ||
| final String key = extractParameter(annotation, String.class, "key") | ||
| .orElseThrow(() -> new ProcessingErrorException("@Mdc annotation must have 'key' attribute", annotation.getAnnotationType().asElement())); |
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.
вроде договорились что он в итоге не обязательный тк имя берется как имя аргумента
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.
Тут смысл в том, что для аннотации над методом оба параметры обязательны. А для аннотации на аргументе уже оба необязательны:
@Mdc // непонятно, что брать в качестве ключа и значения?
void test() {...}
// здесь наоборот, все понятно, вызовем MDC.put("name", name);
void test(@Mdc String name) {...}| } | ||
|
|
||
| private static <T> Optional<T> extractParameter(AnnotationMirror annotation, Class<T> type, String name) { | ||
| return annotation.getElementValues() |
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.
AnnotationUtils.parseAnnotationValueWithoutDefault
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.
Поправил
| currentContextBuilder.addStatement("var __$N = $N.get($S)", key, MDC_CONTEXT_VAR_NAME, key); | ||
| } | ||
| if (value.startsWith("${") && value.endsWith("}")) { | ||
| fillMdcBuilder.addStatement("$T.put($S, $N)", mdc, key, value.substring(2, value.length() - 1)); |
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.
$N - это для имени переменной, а у вас там судя по всему свободный код, для него есть $L.
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.
Поправил
ed6471b to
87ba375
Compare
Реализация задачи #144