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

feat: otel updates for latest spec doc #5

Merged
merged 14 commits into from
Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
/.nyc_output
/docs/
/out/
/build/
**/build/
system-test/secrets.js
system-test/*key.json
*.lock
Expand Down
2 changes: 2 additions & 0 deletions src/publisher/message-queues.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ export abstract class MessageQueue extends EventEmitter {
}
spanMessages.forEach(m => {
tracing.PubsubSpans.updatePublisherTopicName(m.parentSpan!, topic.name);

Choose a reason for hiding this comment

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

Technically not part of this PR but why is this necessary? It looks like the same information is already passed when createPublisherSpan is called.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Mentioned over chat, but the trouble is that, since everything is async, it's possible that the code could get to the part that sets the initial span name before the project ID is resolved. In that case, the project ID will become {{project}} in the final span, which isn't great. The project ID must be resolved by this point, so this just makes sure the right names are in place.

tracing.PubsubEvents.publishStart(m);
});
}

Expand Down Expand Up @@ -143,6 +144,7 @@ export abstract class MessageQueue extends EventEmitter {
// We're finished with both the RPC and the whole publish operation,
// so close out all of the related spans.
rpcSpan?.end();
tracing.PubsubEvents.publishEnd(m);
m.parentSpan?.end();
});
}
Expand Down
17 changes: 17 additions & 0 deletions src/publisher/pubsub-message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,23 @@ export interface PubsubMessage
* @private
*/
publishSchedulerSpan?: tracing.Span;

/**
* If this is a message being received from a subscription, expose the ackId.

Choose a reason for hiding this comment

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

I think expose here just means expose to the other parts of the client library and not users right?

Copy link
Owner Author

@feywind feywind Mar 22, 2024

Choose a reason for hiding this comment

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

Yep. I need to make a whole separate pass of converting @private to @internal since tsdoc changed that.

*
* @private
* @internal
*/
ackId?: string;

/**
* If this is a message being received from a subscription, expose the exactly
* once delivery flag.
*
* @private
* @internal
*/
isExactlyOnceDelivery?: boolean;
}

/**
Expand Down
23 changes: 23 additions & 0 deletions src/subscriber.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ export class SubscriberSpans {
}

// Emit an event for calling modAck.
// Note that we don't currently support users calling modAck directly, but
// this may be used in the future for things like fully managed pull
// subscriptions.
modAckCall(deadline: Duration) {
if (this.processing) {
tracing.PubsubEvents.modAckCalled(this.processing, deadline);
Expand Down Expand Up @@ -243,6 +246,16 @@ export class Message implements tracing.MessageWithAttributes {
*/
parentSpan?: tracing.Span;

/**
* We'll save the state of the subscription's exactly once delivery flag at the
* time the message was received. This is pretty much only for tracing, as we will
* generally use the live state of the subscription to figure out how to respond.
*
* @private
* @internal
*/
isExactlyOnceDelivery: boolean;

/**
* @private
*
Expand Down Expand Up @@ -344,6 +357,14 @@ export class Message implements tracing.MessageWithAttributes {
*/
this.subSpans = new SubscriberSpans(this);

/**
* Save the state of the subscription into the message for later tracing.
*
* @private
* @internal
*/
this.isExactlyOnceDelivery = sub.isExactlyOnceDelivery;

this._handled = false;
this._length = this.data.length;
this._subscriber = sub;
Expand Down Expand Up @@ -423,6 +444,7 @@ export class Message implements tracing.MessageWithAttributes {

/**
* Modifies the ack deadline.
* At present time, this should generally not be called by users.
*
* @param {number} deadline The number of seconds to extend the deadline.
* @private
Expand All @@ -437,6 +459,7 @@ export class Message implements tracing.MessageWithAttributes {
/**
* Modifies the ack deadline, expecting a response (for exactly-once delivery subscriptions).
* If exactly-once delivery is not enabled, this will immediately resolve successfully.
* At present time, this should generally not be called by users.
*
* @param {number} deadline The number of seconds to extend the deadline.
* @private
Expand Down
2 changes: 1 addition & 1 deletion src/subscription.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ import {DebugMessage} from './debug';
export {AckError, AckResponse, AckResponses} from './subscriber';

import {EmitterCallback, WrappingEmitter} from './wrapping-emitter';
import * as tracing from './telemetry-tracing';

export type PushConfig = google.pubsub.v1.IPushConfig;
export type OidcToken = google.pubsub.v1.PushConfig.IOidcToken;
Expand Down Expand Up @@ -1238,6 +1237,7 @@ export class Subscription extends WrappingEmitter {

return formatted as google.pubsub.v1.ISubscription;
}

/*!
* Format the name of a subscription. A subscription's full name is in the
* format of projects/{projectId}/subscriptions/{subName}.
Expand Down
Loading
Loading