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

pass connection_id to replication activity apm tags #21949

Merged
merged 1 commit into from
Jan 27, 2023

Conversation

cgardens
Copy link
Contributor

@cgardens cgardens commented Jan 26, 2023

What

  • I am trying to create a datadog dashboard to measure the throughput of our canary workspace.

How

  • Include connection id in the replication activity APM tags, so I can use it as a filter in datadog.
  • Added connection_id as one of the keys that gets passed in via StandardSyncInput.

@octavia-squidington-iv octavia-squidington-iv added area/platform issues related to the platform area/worker Related to worker labels Jan 26, 2023
.withDestinationResourceRequirements(config.getDestinationResourceRequirements());
.withDestinationResourceRequirements(config.getDestinationResourceRequirements())
.withConnectionId(connectionId)
.withWorkspaceId(config.getWorkspaceId());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed workspace_id wasn't passed in here. But is passed in other places where we build this struct. @mfsiega-airbyte was that intentional?

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed not intentional - but same question as Pedro.

@@ -400,7 +400,9 @@ public TemporalResponse<StandardSyncOutput> submitSync(final long jobId, final i
.withState(config.getState())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed source_id and destination_id wasn't passed in here. But is passed in other places where we build this struct. @pedroslopez was that intentional?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not intentional. I didn't know submitSync was a thing when working on the original PR that introduced them.

I've actually been recently trying to understand where this submitSync method comes into play... when does this get executed? From what I've seen we're always using the inputs as generated by the GenerateInputActivity in actual sync jobs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tbh, i've lost track of it as well. @benmoriceau is this something you understand well?

@cgardens cgardens temporarily deployed to more-secrets January 26, 2023 23:42 — with GitHub Actions Inactive
@cgardens cgardens temporarily deployed to more-secrets January 26, 2023 23:42 — with GitHub Actions Inactive
@cgardens cgardens marked this pull request as ready for review January 26, 2023 23:43
Copy link
Contributor

@jdpgrailsdev jdpgrailsdev left a comment

Choose a reason for hiding this comment

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

:shipit:

@github-actions
Copy link
Contributor

Airbyte Code Coverage

File Coverage [35.3%]
TemporalClient.java 61.44% 🍏
GenerateInputActivityImpl.java 1.19%
ReplicationActivityImpl.java 0.67%
Total Project Coverage 24%

@cgardens cgardens merged commit 0bf9f19 into master Jan 27, 2023
@cgardens cgardens deleted the cgardens/pass-connection-id-to-replicate-activity branch January 27, 2023 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/platform issues related to the platform area/worker Related to worker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants