You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2020/02/21 04:47:51 UTC
[GitHub] [skywalking] dengliming opened a new pull request #4391: Fix
otherMetricsDataTTL config not working
dengliming opened a new pull request #4391: Fix otherMetricsDataTTL config not working
URL: https://github.com/apache/skywalking/pull/4391
Please answer these questions before submitting pull request
- Why submit this pull request?
- [ ] Bug fix
- [ ] New feature provided
- [ ] Improve performance
- Related issues
___
### Bug fix
- Bug description.
There is no setter of otherMetricsDataTTL method in StorageModuleElasticsearchConfig.This can lead to dayMetrics only stored 2 days default.
See #4388
- How to fix?
Add setter annotation.
___
### New feature or improvement
- Describe the details and related test reports.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] codecov-io commented on issue #4391: Fix
otherMetricsDataTTL config not working
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4391: Fix otherMetricsDataTTL config not working
URL: https://github.com/apache/skywalking/pull/4391#issuecomment-589505091
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4391?src=pr&el=h1) Report
> Merging [#4391](https://codecov.io/gh/apache/skywalking/pull/4391?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/d02e472d14eda62b24ec16bb679baa2bc3b986f6?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `0%`.
[](https://codecov.io/gh/apache/skywalking/pull/4391?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4391 +/- ##
=======================================
Coverage 25.44% 25.44%
=======================================
Files 1205 1205
Lines 27850 27850
Branches 3838 3838
=======================================
Hits 7087 7087
Misses 20123 20123
Partials 640 640
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4391?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...lasticsearch/StorageModuleElasticsearchConfig.java](https://codecov.io/gh/apache/skywalking/pull/4391/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvU3RvcmFnZU1vZHVsZUVsYXN0aWNzZWFyY2hDb25maWcuamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4391?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4391?src=pr&el=footer). Last update [d02e472...94c93fa](https://codecov.io/gh/apache/skywalking/pull/4391?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] dengliming edited a comment on issue #4391: Fix
otherMetricsDataTTL config not working
Posted by GitBox <gi...@apache.org>.
dengliming edited a comment on issue #4391: Fix otherMetricsDataTTL config not working
URL: https://github.com/apache/skywalking/pull/4391#issuecomment-589497738
> I remember setter is not required, are you sure this will be an issue?
There is only Getter annotation in class,So if the field you need to provide setter method should add Setter annotation.And I also think that is the reason for this problem. [https://github.com/apache/skywalking/issues/4388](https://github.com/apache/skywalking/issues/4388),It is a bug(I think)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4391: Fix
otherMetricsDataTTL config not working
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4391: Fix otherMetricsDataTTL config not working
URL: https://github.com/apache/skywalking/pull/4391#issuecomment-589507875
This is the config of the official DEMO env using. It works as expected.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng closed pull request #4391: Fix
otherMetricsDataTTL config not working
Posted by GitBox <gi...@apache.org>.
wu-sheng closed pull request #4391: Fix otherMetricsDataTTL config not working
URL: https://github.com/apache/skywalking/pull/4391
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] dengliming commented on issue #4391: Fix
otherMetricsDataTTL config not working
Posted by GitBox <gi...@apache.org>.
dengliming commented on issue #4391: Fix otherMetricsDataTTL config not working
URL: https://github.com/apache/skywalking/pull/4391#issuecomment-589505527
Thanks for your detailed explanation.It's my misunderstanding.Digression If i want to keep more days trace data.How to do it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4391: Fix
otherMetricsDataTTL config not working
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4391: Fix otherMetricsDataTTL config not working
URL: https://github.com/apache/skywalking/pull/4391#issuecomment-589502931
My point is, the codes you posted are not related to trace TTL at all. That is my question.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4391: Fix
otherMetricsDataTTL config not working
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4391: Fix otherMetricsDataTTL config not working
URL: https://github.com/apache/skywalking/pull/4391#issuecomment-589507668
My local test confirmed, this value could be set without `@setter`. Also, according to code `ModuleDefine#copyProperties`, it is using refactor to access field, rather than set method.
So, this is not the issue. even this is not an issue generally, no matter related to your question or not
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] dengliming commented on issue #4391: Fix
otherMetricsDataTTL config not working
Posted by GitBox <gi...@apache.org>.
dengliming commented on issue #4391: Fix otherMetricsDataTTL config not working
URL: https://github.com/apache/skywalking/pull/4391#issuecomment-589496929
Why need? Please check the code. you will see.
org.apache.skywalking.oap.server.storage.plugin.elasticsearch.StorageModuleElasticsearchConfig#getDayMetricsDataTTL
```
public int getDayMetricsDataTTL() {
if (otherMetricsDataTTL > 0) {
return otherMetricsDataTTL;
}
return dayMetricsDataTTL;
}
```
If otherMetricsDataTTL config not working, otherMetricsDataTTL is always be zero,Then there is no
meaning here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4391: Fix
otherMetricsDataTTL config not working
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4391: Fix otherMetricsDataTTL config not working
URL: https://github.com/apache/skywalking/pull/4391#issuecomment-589497063
I remember setter is not required, are you sure this will be an issue?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4391: Fix
otherMetricsDataTTL config not working
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4391: Fix otherMetricsDataTTL config not working
URL: https://github.com/apache/skywalking/pull/4391#issuecomment-589499425
I will check later. Currently, I am not sure.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4391: Fix
otherMetricsDataTTL config not working
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4391: Fix otherMetricsDataTTL config not working
URL: https://github.com/apache/skywalking/pull/4391#issuecomment-589500449
In that PR, you talked about trace TTL, but this one has nothing related to that, no matter it is a big. Are you are the thing? I doubt it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] dengliming commented on issue #4391: Fix
otherMetricsDataTTL config not working
Posted by GitBox <gi...@apache.org>.
dengliming commented on issue #4391: Fix otherMetricsDataTTL config not working
URL: https://github.com/apache/skywalking/pull/4391#issuecomment-589501877
> In that PR, you talked about trace TTL, but this one has nothing related to that, no matter it is a big. Are you are the thing? I doubt it.
I don't figure out why my setting not working,It seems only keep two days trace data.So I checked the code.And I'm not boring!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4391: Fix
otherMetricsDataTTL config not working
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4391: Fix otherMetricsDataTTL config not working
URL: https://github.com/apache/skywalking/pull/4391#issuecomment-589503106
getDayMetricsDataTTL methd is for metrics ttl at the day precision level.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] dengliming commented on issue #4391: Fix
otherMetricsDataTTL config not working
Posted by GitBox <gi...@apache.org>.
dengliming commented on issue #4391: Fix otherMetricsDataTTL config not working
URL: https://github.com/apache/skywalking/pull/4391#issuecomment-589497738
> I remember setter is not required, are you sure this will be an issue?
I think is the reason for this problem. [https://github.com/apache/skywalking/issues/4388](https://github.com/apache/skywalking/issues/4388),It is a bug(I think)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services