Skip to content

upgrade log throwable - #74

Merged
kevinten10 merged 30 commits into
masterfrom
feature/metrics
Jan 5, 2022
Merged

upgrade log throwable#74
kevinten10 merged 30 commits into
masterfrom
feature/metrics

Conversation

@dgzpg

@dgzpg dgzpg commented Dec 20, 2021

Copy link
Copy Markdown
Contributor

Description

upgrade log throwable

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

@codecov-commenter

codecov-commenter commented Dec 20, 2021

Copy link
Copy Markdown

Codecov Report

Merging #74 (236a374) into master (384cc0f) will decrease coverage by 0.78%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #74      +/-   ##
============================================
- Coverage     33.92%   33.14%   -0.79%     
+ Complexity       70       69       -1     
============================================
  Files            25       25              
  Lines           731      706      -25     
  Branches         74       69       -5     
============================================
- Hits            248      234      -14     
+ Misses          468      458      -10     
+ Partials         15       14       -1     
Impacted Files Coverage Δ
...xcloud/capa/spi/aws/config/AwsCapaConfigStore.java 65.80% <0.00%> (-5.98%) ⬇️
...loud/capa/spi/aws/config/entity/Configuration.java 60.60% <0.00%> (-0.69%) ⬇️
...s/telemetry/metrics/CloudWatchMetricsExporter.java 0.00% <0.00%> (ø)
...up/rxcloud/capa/spi/aws/mesh/http/AwsCapaHttp.java 46.42% <0.00%> (+24.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 384cc0f...236a374. Read the comment docs.

}
}
} catch (Throwable throwable) {
throwable.printStackTrace();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you explain the usage here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

能够输出异常日志

private static void putLogToCloudWatch(String message, Map<String, String> tags) {
if (LogConfiguration.containsKey(CLOUD_WATCH_AGENT_SWITCH_NAME)
&& Boolean.TRUE.toString().equalsIgnoreCase(LogConfiguration.get(CLOUD_WATCH_AGENT_SWITCH_NAME))) {
//put logs by agent

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

// 后面接空格

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

好的,我改下

if (message != null) {
pw.println(message);
}
throwable.printStackTrace(pw);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

能否把printStackTrace这种用法统一包装起来,这样后续也好替换

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

可以考虑像这种异常日志使用一套规范去打印,如日志信息里面加上日志错误级别。

if(StringUtils.isNotBlank(appId)){
defaultTags.put(APP_ID_NAME, appId);
}
}catch (Throwable e){

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

这里catch的目的是?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

这里只是增强的加一下tag,不强依赖这些tag,如果如appId获取失败不会导致整个日志输出失败。

collectedMetrics.forEach(CloudWatchMetricsExporter::convertAndSend);
collectedMetrics.forEach(CloudWatchMetricsExporter::convertAndSend);
} catch (Throwable e) {
log.warn("Fail to export metrics.", e);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

中间件内部的异常日志,最好统一加个前缀,例如:[Capa.Log]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

JasmineJ1230 and others added 2 commits December 30, 2021 16:01
@kevinten10
kevinten10 merged commit c33438f into master Jan 5, 2022
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.

7 participants