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

[core] Update OpenCensus #32553

Merged
merged 27 commits into from
Feb 25, 2023
Merged

[core] Update OpenCensus #32553

merged 27 commits into from
Feb 25, 2023

Conversation

fishbone
Copy link
Contributor

@fishbone fishbone commented Feb 14, 2023 •
edited
Loading

Why are these changes needed?

This PR updated OpenCensus.

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@fishbone
up
6b93b76
@fishbone
check
18c8bb9
@fishbone
up
2c283fd
@fishbone
Merge branch 'update-boost' into update-opc
e70e58a
@fishbone
up
5e70f05
@fishbone
fix win
31cad40
@fishbone
try to fix arm7
9627994
@fishbone
[build_base] force
4c0da14
@fishbone
fix
90c00dc
@fishbone
[build_base] force
d3b6a47
@fishbone
Merge remote-tracking branch 'upstream/master' into update-boost
5d04113
@fishbone
[build_base] for base building
7524256
@fishbone
[build_base] try to make base rebuild
31064f0
@fishbone
Merge remote-tracking branch 'upstream/master' into update-boost
e2ca748 
Signed-off-by: Yi Cheng <74173148+iycheng@users.noreply.github.com>
@fishbone
[build_base] force build baase
08c9600 
Signed-off-by: Yi Cheng <74173148+iycheng@users.noreply.github.com>
@fishbone
Merge branch 'update-boost' into update-opc
e3e4fa2 
Signed-off-by: Yi Cheng <74173148+iycheng@users.noreply.github.com>
@fishbone
up
c1c83a6 
Signed-off-by: Yi Cheng <74173148+iycheng@users.noreply.github.com>
@fishbone
up
c7ff232 
Signed-off-by: Yi Cheng <74173148+iycheng@users.noreply.github.com>
@fishbone
fix
c3e6a38 
Signed-off-by: Yi Cheng <74173148+iycheng@users.noreply.github.com>
@fishbone
fix
b15f687 
Signed-off-by: Yi Cheng <74173148+iycheng@users.noreply.github.com>
@fishbone
fix win
b2bbff2 
Signed-off-by: Yi Cheng <74173148+iycheng@users.noreply.github.com>
@fishbone
Merge branch 'update-boost' into update-opc
16a70ca 
Signed-off-by: Yi Cheng <74173148+iycheng@users.noreply.github.com>
@fishbone
[build_base] rebuild base
2809b7f 
Signed-off-by: Yi Cheng <74173148+iycheng@users.noreply.github.com>
@fishbone
Merge remote-tracking branch 'upstream/master' into update-opc
9bb1105 
Signed-off-by: Yi Cheng <74173148+iycheng@users.noreply.github.com>
@fishbone fishbone changed the title [wip]Update opc [core] Update OpenCensus Feb 22, 2023
@fishbone fishbone marked this pull request as ready for review February 22, 2023 23:54
@fishbone
fix
37fb665 
Signed-off-by: Yi Cheng <74173148+iycheng@users.noreply.github.com>
@fishbone
Merge remote-tracking branch 'upstream/master' into update-opc
cd380ed 
Signed-off-by: Yi Cheng <74173148+iycheng@users.noreply.github.com>
@fishbone
Merge remote-tracking branch 'upstream/master' into update-opc
21c45ea 
Signed-off-by: Yi Cheng <74173148+iycheng@users.noreply.github.com>
rkooo567
rkooo567 reviewed Feb 24, 2023
View reviewed changes
Copy link
Contributor

@rkooo567 rkooo567 left a comment

Choose a reason for hiding this comment

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

Didn't know they still release new versions... thought it was deprecated.

Btw, how old is current version, and is the new version the latest stable release?

Also, little concerned about that there are lots of changes from the patch file. Is the only relevant changes from the changes from the upstream code?

thirdparty/patches/opencensus-cpp-shutdown-api.patch
class PrometheusExporter final : public ::prometheus::Collectable {
public:
- std::vector<prometheus::MetricFamily> Collect() const override;
+ std::vector<prometheus::MetricFamily> Collect() override;
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious, but why is it changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because prometheus-cpp mark it as const in the new version and this one update the code to make it compile. Don't want to upgrade prometheus-cpp since the changes are too big and seems not a blocker for std-20

@fishbone
Copy link
Contributor Author

Also, little concerned about that there are lots of changes from the patch file. Is the only relevant changes from the changes from the upstream code?

It's because code file changes invalidates the patch files. Basically, I just copy paste what's in the patch files and regenerate it.

The old one is 2019 and the new one is 2022. I don't think it's released any more and I think they are living on the master. Not too many commits between these two actually. If the test looks good, the risk should be low.

rkooo567
rkooo567 approved these changes Feb 25, 2023
View reviewed changes
Copy link
Contributor

@rkooo567 rkooo567 left a comment

Choose a reason for hiding this comment

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

LGTM. Finger crossed this doesn't cause random issues..

@fishbone fishbone merged commit 2e83d1b into ray-project:master Feb 25, 2023
@pcmoritz
Copy link
Contributor

Can you make a PR against OpenCensus suggesting our patches and the reasoning why they can be beneficial to other users as well (if they can be)? This will reduce our maintainance overhead in the future.

edoakes pushed a commit to edoakes/ray that referenced this pull request Mar 22, 2023
@fishbone @edoakes
[core] Update OpenCensus (ray-project#32553)
496373e 
This PR updated OpenCensus.

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
peytondmurray pushed a commit to peytondmurray/ray that referenced this pull request Mar 22, 2023
@fishbone @peytondmurray
[core] Update OpenCensus (ray-project#32553)
03b20a2 
This PR updated OpenCensus.
elliottower pushed a commit to elliottower/ray that referenced this pull request Apr 22, 2023
@fishbone @elliottower
[core] Update OpenCensus (ray-project#32553)
deac2fc 
This PR updated OpenCensus.

Signed-off-by: elliottower <elliot@elliottower.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rkooo567 rkooo567 rkooo567 approved these changes

Assignees

@scv119 scv119

@rkooo567 rkooo567

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fishbone @pcmoritz @rkooo567 @scv119