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.

Sign up for GitHub

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

Jump to bottom

Limit span names/labels to service limits #345

Merged
merged 1 commit into from
Jan 14, 2017
Merged

Limit span names/labels to service limits #345

merged 1 commit into from
Jan 14, 2017
+75 −4

Conversation

matthewloring
Copy link
Contributor

No description provided.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 13, 2017
@matthewloring
Copy link
Contributor Author

PTAL.

ofrobots
ofrobots suggested changes Jan 13, 2017
View reviewed changes
src/trace-span.js
var val = '' + value;
// The official limit imposed by the service is 16k per label value.
// We truncate smaller than that to keep requests small.
if (val.length > 5000) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@matthewloring matthewloring changed the title Limit label values to 5000 characters Limit span names/labels to service limits Jan 13, 2017
@matthewloring matthewloring mentioned this pull request Jan 13, 2017
Closed
ofrobots
ofrobots suggested changes Jan 14, 2017
View reviewed changes
src/constants.js
TRACE_SERVICE_LABEL_KEY_LIMIT: 128,

/** Maximum size of a label value in bytes. */
TRACE_SERVICE_LABEL_VALUE_LIMIT: 16 * 1000,

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/trace-span.js
/**
* Creates a trace span object.
* @constructor
*/
function TraceSpan(name, spanId, parentSpanId) {
this.name = name;
if (name.length > constants.TRACE_SERVICE_SPAN_NAME_LIMIT) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/trace-span.js
@@ -37,7 +43,14 @@ function TraceSpan(name, spanId, parentSpanId) {
* @param {string} value The new value of the label.
*/
TraceSpan.prototype.setLabel = function(key, value) {
this.labels[key] = '' + value;
if (key.length > constants.TRACE_SERVICE_LABEL_KEY_LIMIT) {

This comment was marked as spam.

Sign in to view

ofrobots
ofrobots reviewed Jan 14, 2017
View reviewed changes
src/trace-span.js
@@ -17,13 +17,26 @@
'use strict';

var util = require('util');
var constants = require('./constants.js');

function truncate(string, length) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

src/constants.js
TRACE_OPTIONS_TRACE_ENABLED: 1 << 0,

/** See: cloud.google.com/trace/api/reference/rest/v1/projects.traces for limits. */
/** Maximum size of a span name in bytes. */

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@matthewloring matthewloring merged commit 94e5dad into googleapis:master Jan 14, 2017
@matthewloring matthewloring deleted the label-size branch January 14, 2017 02:08
vmarchaud pushed a commit to keymetrics/origa that referenced this pull request Jan 16, 2017
@matthewloring @vmarchaud
[PATCH] Limit span names/labels to service limits (#345)
fecddf1 
PR-URL: googleapis/cloud-trace-nodejs#345
@ofrobots ofrobots mentioned this pull request Jan 21, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ofrobots ofrobots ofrobots approved these changes

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@matthewloring @ofrobots @googlebot