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

Census Tag Set API and implementation #4750

Merged
merged 16 commits into from
Jan 22, 2016
Merged

Conversation

a-veitch
Copy link
Contributor

No description provided.

@grpc-kokoro
Copy link

Can one of the admins verify this patch?

1 similar comment
@grpc-kokoro
Copy link

Can one of the admins verify this patch?

/* Get a contexts tag set. */
census_tag_set *census_context_tag_set(census_context *context);
/* A tag is a key:value pair. The key is a non-empty, printable, nil-terminated
string. The value is a binary string, that may be printable. There are no

Choose a reason for hiding this comment

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

maybe be more specific about printable? that may be printable (ex. UTF-8 decoded).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

/* Maximum length of a tag's key or value. */
#define CENSUS_MAX_TAG_KV_LEN 255
/* Maximum number of propagatable tags. */
#define CENSUS_MAX_PROPAGATED_TAGS 255

Choose a reason for hiding this comment

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

Should we start with a lower number? Also please mention the hard limit for this implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could have a lower number, but why? Hard limit comment has been added to the implementation detail (I want to keep it out of the API).

@a-veitch
Copy link
Contributor Author

Fixes #4665

#include "src/core/support/string.h"

// Functions in this file support the public tag_set API, as well as
// encoding/decoding tag_sets as part of context transmission across

Choose a reason for hiding this comment

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

s/transmission/propagation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@a-veitch
Copy link
Contributor Author

Have updated: buf_size no longer used for in and out args,

bogdandrutu pushed a commit that referenced this pull request Jan 22, 2016
Census Tag Set API and implementation
@bogdandrutu bogdandrutu merged commit fc65bf0 into grpc:master Jan 22, 2016
@a-veitch a-veitch deleted the tag_set branch January 26, 2016 00:55
@lock lock bot locked as resolved and limited conversation to collaborators Jan 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants