Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Support OAP server v9 Core concept: Layer, backend side #8367

Closed
wants to merge 7 commits into from
Closed

Support OAP server v9 Core concept: Layer, backend side #8367

wants to merge 7 commits into from

Conversation

wankai123
Copy link
Member

  • If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #.
  • Update the CHANGES log.

related to: #8241

@wankai123 wankai123 added the backend OAP backend related. label Dec 30, 2021
@wankai123 wankai123 added this to the 9.0.0 milestone Dec 30, 2021
@wankai123 wankai123 added the core feature Core and important feature. Sometimes, break backwards compatibility. label Dec 30, 2021
@wu-sheng
Copy link
Member

Generally, this PR is good. There are several e2e verifications failed, please recheck.
The following things are required to change

  1. The Change Log. This change is adding a new concept, and break some parts from v8, such as MySQL/PostgreSQL/H2(I am not sure for InfluxDB and IoTDB) are not working. We need change log to tell users which tables(of which storages)(service_traffic, instance_traffic or more?) are required to removed before changing to v9 OAP. Also, we should mention all(?) metrics are compatible with previous data format.
  2. scope-definitions.md doc should be updated according. Also the change log should include it.
  3. mal.md doc should be updated for new method parameters of downsampling functions.
  4. lal.md doc should be updated for new layer method in extractor.
  5. overview.md doc should be updated, layer becomes an important concept for the end user.
  6. A FAQ should be added to cover details in (1). The change log should not be too long.
  7. Log-Data-Protocol.md should be updated as new layer field is added as optional.

TBD, @mrproliu We need some helps on CLI side once this PR gets merged

  1. e2e previous service verification should add layer check. We have done compatible upgrade Server(GraphQL) object to v2, so, old getAllServices or search* should support layer once the backend version is v9.(According to GraphQL service org.apache.skywalking.oap.query.graphql.resolver.Query.version).
  2. New metadata query should be added, especially listServices(layer: String!): [Service!]!.

@wu-sheng wu-sheng mentioned this pull request Dec 31, 2021
Closed
kezhenxu94
kezhenxu94 requested changes Dec 31, 2021
View reviewed changes
Copy link
Member

@kezhenxu94 kezhenxu94 left a comment

Choose a reason for hiding this comment

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

Just did a first-round review

oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/Layer.java Outdated Show resolved Hide resolved
oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/Layer.java Outdated Show resolved Hide resolved
...ver-core/src/main/java/org/apache/skywalking/oap/server/core/query/MetadataQueryService.java Outdated Show resolved Hide resolved
.../org/apache/skywalking/oap/server/storage/plugin/elasticsearch/query/MetadataQueryEsDAO.java Show resolved Hide resolved
...rver/server-core/src/main/java/org/apache/skywalking/oap/server/core/query/type/Service.java Outdated Show resolved Hide resolved
...ain/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/dao/H2MetadataQueryDAO.java Show resolved Hide resolved
...ore/src/main/java/org/apache/skywalking/oap/server/core/storage/query/IMetadataQueryDAO.java Show resolved Hide resolved
@kezhenxu94
Copy link
Member

TBD, @mrproliu We need some helps on CLI side once this PR gets merged

  1. e2e previous service verification should add layer check. We have done compatible upgrade Server(GraphQL) object to v2, so, old getAllServices or search* should support layer once the backend version is v9.(According to GraphQL service org.apache.skywalking.oap.query.graphql.resolver.Query.version).
  2. New metadata query should be added, especially listServices(layer: String!): [Service!]!.

cc @fgksgf

@wu-sheng
Copy link
Member

wu-sheng commented Jan 1, 2022

The CI and e2e passed.Please follow other review comments. Once you are done, please ping me.

@wu-sheng
Copy link
Member

wu-sheng commented Jan 3, 2022

Codes seem good to me. Tests are passed. Let's update the doc before merging.

@wu-sheng wu-sheng marked this pull request as ready for review January 4, 2022 06:31
@wu-sheng
Copy link
Member

wu-sheng commented Jan 4, 2022 •
edited
Loading

Zhenxu has some personal stuff for the next 2 weeks. @wankai123 Please open another pull request in order to make sure I can merge that, as I don't want to ping him these days.

@wankai123
Copy link
Member Author

Submit a new PR for this

@wankai123 wankai123 closed this Jan 4, 2022
@wankai123 wankai123 mentioned this pull request Jan 4, 2022
Merged
6 tasks
@kezhenxu94 kezhenxu94 mentioned this pull request Feb 16, 2022
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wu-sheng wu-sheng wu-sheng left review comments

@kezhenxu94 kezhenxu94 Awaiting requested review from kezhenxu94

Assignees
No one assigned
Labels
backend OAP backend related. core feature Core and important feature. Sometimes, break backwards compatibility.
Projects
None yet
Milestone
9.0.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@wankai123 @wu-sheng @kezhenxu94