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

#229: NullReferenceException in TelemetryDispatchMessageInspector.BeforeSendReply when operation is OneWay #237

Merged
merged 11 commits into from
Mar 17, 2022

Conversation

ievgen-platonov
Copy link
Contributor

See details in the issue #229 (#229)

@ievgen-platonov ievgen-platonov requested a review from a team March 15, 2022 22:28
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Mar 15, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@cijothomas
Copy link
Member

cijothomas commented Mar 15, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@CodeBlanch
Copy link
Member

@ievgen-platonov This looks good to me, thanks for the contribution! Would you mind updating the CHANGELOG? You can add a section for "1.0.0-rc6" that's the version I'll use to release the fix.

@codecov
Copy link

codecov bot commented Mar 16, 2022

Codecov Report

Merging #237 (1295310) into main (8b456b4) will not change coverage.
The diff coverage is n/a.

❗ Current head 1295310 differs from pull request most recent head bf4af7a. Consider uploading reports for the commit bf4af7a to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #237   +/-   ##
=======================================
  Coverage   79.14%   79.14%           
=======================================
  Files          97       97           
  Lines        2498     2498           
=======================================
  Hits         1977     1977           
  Misses        521      521           

@IevgenPlatonov
Copy link
Contributor

@CodeBlanch Sure. Done.

@CodeBlanch
Copy link
Member

@iepl Hey it looks like the markdownlint task failed on your CHANGELOG update. Needs a blank line like...

## 1.0.0-rc6

* Fixed a 'NullReferenceException' in
  'TelemetryDispatchMessageInspector.BeforeSendReply' when operation is OneWay
  ([#237](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/237))

I tried to fix it for you but it looks like I don't have access to your branch.

@IevgenPlatonov
Copy link
Contributor

@CodeBlanch Fixed. I added some extra asserts as well. Just copied from existing test.

Copy link
Member

@CodeBlanch CodeBlanch left a comment

Choose a reason for hiding this comment

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

LGTM

@CodeBlanch CodeBlanch merged commit 954b81c into open-telemetry:main Mar 17, 2022
swetharavichandrancisco pushed a commit to swetharavichandrancisco/opentelemetry-dotnet-contrib that referenced this pull request Apr 4, 2022
…eInspector.BeforeSendReply when operation is OneWay (open-telemetry#237)

* Update TelemetryDispatchMessageInspector.cs

open-telemetry#229: Check if reply is not null

* open-telemetry#229: Added TelemetryDispatchMessageInspectorForOneWayOperationsTests

* open-telemetry#229: Rename parameters

* open-telemetry#229: Added new "1.0.0-rc6" version to CHANGELOG.md

* open-telemetry#229: Fix text formatting

* open-telemetry#229: Fix CHANGELOG formatting. Added extra asserts to the test

* open-telemetry#229: Fix formatting

* open-telemetry#229: Fix formatting in GHANGELOG.md

Co-authored-by: iepl <platonov.evgen@gmail.com>
Co-authored-by: Ievgen Platonov <77076677+iepl@users.noreply.github.com>
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.

4 participants