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.

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

Add OpenTelemetry #153

Closed
wants to merge 2 commits into from
Closed

Conversation

atoulme
Copy link
Contributor

@atoulme atoulme commented Dec 24, 2020

Signed-off-by: Antoine Toulme antoine@lunar-ocean.com

This adds OpenTelemetry to Fabric Java chaincode.

@atoulme atoulme requested a review from a team as a code owner December 24, 2020 06:32
@atoulme atoulme force-pushed the add_opentelemetry branch 8 times, most recently from 24e8bac to 2b4001f Compare December 24, 2020 08:20
@jt-nti
Copy link
Member

jt-nti commented Jan 5, 2021

Hi @atoulme, thanks very much for your pull request. Adding metrics to Fabric looks like it could be really useful and I think it would be good to get this proposal reviewed by the wider Fabric community.

There's a Request For Comments process which would need to be followed before merging any PRs, however it would probably be a good idea discussing on Rocket Chat and on one of the maintainer calls beforehand. For example to discuss the choice of OpenTelemetry/B3 headers over alternatives etc.

From an implementation point of view, I noticed that the protobuf code had been updated, but that wasn't reflected in a change to the protos project, or the peer code.

Thanks again, and I look forward to hearing from you about taking this proposal forward. James

@atoulme
Copy link
Contributor Author

atoulme commented Jan 5, 2021

Thanks James, I look forward to working with you on this!

Signed-off-by: Antoine Toulme <antoine@lunar-ocean.com>
Signed-off-by: Antoine Toulme <antoine@lunar-ocean.com>
@atoulme
Copy link
Contributor Author

atoulme commented Feb 11, 2021

The RFC is open here for further discussion: hyperledger/fabric-rfcs#42

@jt-nti
Copy link
Member

jt-nti commented Mar 1, 2021

@atoulme I'm hoping that your RFC will get a mention at the next maintainers call to get some more visibility

@atoulme
Copy link
Contributor Author

atoulme commented Mar 1, 2021

That'd be swell. I've been busy elsewhere, but I can try to join the call to explain what this is all about.

@jt-nti
Copy link
Member

jt-nti commented Mar 2, 2021

Base automatically changed from master to main March 22, 2021 12:59
@mbwhite
Copy link
Member

mbwhite commented Jun 7, 2021

Closing the PR, note because this is a bad idea, but really needs the RFC as discussed above.

@mbwhite mbwhite closed this Jun 7, 2021
@atoulme atoulme mentioned this pull request Sep 15, 2021
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.

None yet

3 participants