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

Support RequestBody#contentLength. #1227

Merged
merged 4 commits into from
Apr 28, 2021

Conversation

jkozlowski
Copy link
Contributor

@jkozlowski jkozlowski commented Apr 28, 2021

Before this PR

No way to specify content length of body directly.

After this PR

==COMMIT_MSG==
RequestBody supports optional content length.
==COMMIT_MSG==

Possible downsides?

More API. It's kinda already API, for feign replacement, so likely makes sense to support directly. Also useful for #1222

@changelog-app
Copy link

changelog-app bot commented Apr 28, 2021

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

RequestBody supports optional content length.

Check the box to generate changelog(s)

  • Generate changelog entry

@policy-bot policy-bot bot requested a review from CRogers April 28, 2021 11:23
@jkozlowski jkozlowski removed the request for review from CRogers April 28, 2021 11:24
{"traceId":"3519228122fee79e","parentSpanId":"b9eede02cfc9a230","spanId":"fc414565b6a32ddd","type":"LOCAL","operation":"Dialogue-request-attempt initial","startTimeMicroSeconds":1619611357982666,"durationNanoSeconds":1428971,"metadata":{}}
{"traceId":"3519228122fee79e","parentSpanId":"76a3f96c2bd32ebb","spanId":"257e95ea327f70b4","type":"LOCAL","operation":"Dialogue-RetryingChannel","startTimeMicroSeconds":1619611357020816,"durationNanoSeconds":964010756,"metadata":{"serviceName":"service","endpointName":"endpoint","failures":"5","channel":"my-channel"}}
{"traceId":"3519228122fee79e","parentSpanId":null,"spanId":"4791d408917316f4","type":"LOCAL","operation":"Dialogue: request service#endpoint","startTimeMicroSeconds":1619611357020205,"durationNanoSeconds":970171438,"metadata":{"endpointService":"service","endpointName":"endpoint","http.method":"POST","channel":"my-channel","mesh":"false","outcome":"failure","http.status_code":"429"}}
{"traceId":"3519228122fee79e","parentSpanId":"866a99f10ab0c9b9","spanId":"cff8c71cc0a4de65","type":"LOCAL","operation":"dialogue-RetryingChannel-scheduler","startTimeMicroSeconds":1619611357982457,"durationNanoSeconds":8481191,"metadata":{}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thoughts on what happened?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Feels like this should've been there from the start?

Copy link
Contributor

Choose a reason for hiding this comment

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

My guess is that the scheduler runnable is still executing while the test completes, which allows the test to record spans up to, but not including the final dialogue-RetryingChannel-scheduler.
In both before and after, we have the correct number of Dialogue-request-attempt and Dialogue-request-attempt initial spans.

@bulldozer-bot bulldozer-bot bot merged commit 89926b7 into develop Apr 28, 2021
@bulldozer-bot bulldozer-bot bot deleted the jakubk/request-body-content-length branch April 28, 2021 12:20
@svc-autorelease
Copy link
Collaborator

Released 1.120.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants