Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
End-to-end example applications #936
End-to-end example applications #936
Changes from 1 commit
41b1921
d899d2a
d3b2e51
203cad3
38a2a5b
5fc6e6f
c75f113
bbbdb09
0ab8939
b58a421
5248c04
834503e
6191d9d
c38f060
225fadf
ba436f0
403faae
59da6cd
b1e5de9
9682076
4d51283
c925fa2
807d0ea
c2d0484
413a56b
de660c6
a647188
c38945d
7f0de78
bbdb926
0d1a90a
ccc0847
51784cf
437a990
2d8bade
dcf093a
dc654ad
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we switch username & password to also use EnvVar if available? Just to be good internet citizens and encourage people to not hardcode their credentials.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Good point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spec says operationName should be
<destination name> <operation name>
example$"{QueueName} send"
for publish here. I just noticed that yesterday it might be new?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah cool, great call out. I'm really glad to see there's guidance here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a followup PR, we should simply get zipkinhostname from this.Configuration. And set this in appsettings.json.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is namely for when running with docker-compose. I can double-check, but the Zipkin hostname is different as seen by the other containers as it is when hitting it at localhost from a browser.