I'd like the steps to receive an integration-specific sub-type of the IntegrationStepExectionContext, which has integration-specific properties on it used across the steps. This should happen once before the steps are called, and the executionHandler should be typed to receive the integration-specific context.

A concrete use case: initializing a single provider API client object. Without this mechanism, each integration has to invent a way to share a single instance of an API client across the steps.

It would nice to be able to access the utilities without having to dig into other __tests__ directories.

There's a simple trick that can be done to have node resolve the directory without having to muck with require (see this test and this directory)

Once we move to a monorepo, we could just put the utils and fixtures in a separate package that won't be published.

In the Qualys integration, it was necessary to collect QID values (Qualys ID values) from multiple collectors/steps and then at the end we need to batch fetch all of the Qualys vulnerabilities using those accumulated QID values. That is we need some way to collect QID values across multiple steps and share that state with another step that will actually go fetch the Qualys vulnerabilities.

I'm open to different ideas for how to solve this problem as well. In this specific Qualys example, it seems necessary to accumulate a set of values (to avoid duplicates) and then wait until the end to actually fetch the entities from that set.

Integrations that take a few minutes to collect data look like they're doing nothing. Consider logging events for things like:

• Validating configuration
• Starting step X
• Completed step X
• Synchronizing changes

Switching to a monorepo will help keep testing and cli code out of the final package/bundle that we hope to use in deployed environments while still keeping packages nicely in sync.

The code is already pretty well segregated so creating separate packages should be pretty easy.

src/cli -> @jupiterone/integration-sdk-cli
src/framework -> @jupiterone/integration-sdk (or maybe @jupiterone/integration-sdk-core?)
src/testing -> @jupiterone/integration-sdk-testing

Since we might not always get complete data back from integration, we should avoid enforcing schema validation when deployed to production environment. Schema validation should only occur when developing and testing integration.

For example, see the field role below:

{
  "username": {
    "type": "string"
  },
  "account": {
    "type": "string"
  },
  "password": {
    "type": "string",
    "mask": true
  },
  "role": {
     "type": "string",
     "optional": true,
     "default": "jupiterone"
  }
}

I've noticed that integrations are inconsistent in what they return/do in the validation function. I think we need this to be more consistent. In jupiter-managed-integration-sdk, the function did not return anything, but expected one or other error thrown, such as https://bitbucket.org/lifeomic/jupiter-managed-integration-sdk/src/858886f3f9d71eecb407cae153991bf711491e0f/src/integration/types/errors.ts#lines-55, because a simple true or false isn't very useful for communicating why the invocation isn't valid. Of course, we could allow for returning more than just true or false, to include some kind of reason, such as [false, "Authentication failed (path=/thing/I/checked, code=401)"] or [false, "Authorization forbidden (path=/resource/I/tried, code=403"].

export declare function createIntegrationRelationship(options: DirectRelationshipOptions | DirectRelationshipLiteralOptions | MappedRelationshipOptions | MappedRelationshipLiteralOptions): Relationship

The union of so many types makes usage of the function hard to understand. We should consider decomposing into separate functions.

I see this in our logs at runtime:

TypeScript files detected. Registering ts-node.

We should not be using ts-node with our compiled integrations.

The most of the logic already exists in the collect command, we just need pull that out. For the sync command, we will probably need to adapt the code that determines partial datasets from a collection run to work with it.

Include the integration job id at the start of job runs so that users that experience some kind of unexpected issue can pass that along in support requests.

Instead of auto-loading steps, we should move toward having a single index file that contains metadata and imports steps (instead of scanning directory).

Currently, integrations perform a lot of basic validation on IntegrationInstance config that could be shifted to automatic validation in the SDK. We could do this by changing our existing integrationConfigFields.json to be JSON schema compatible.

For example, many integrations have validation like this:

const apiKey = instance.config?.apiKey;
if (!apiKey) {
  throw new Error(
    'Configuration option "apiKey" is missing on the integration instance config',
  );
}

// More simple validation...

Proposed:

integrationConfigFields.json:

{
  // We assume that every `integrationConfigFields.json` is type "object". If it is specified, we
  // will just ignore it.
  // type: "object",
  "properties": {
    "apiKey": {
      "type": "string",
      "format": "masked"
    },
    "myNumberField": {
      "type": "number",
      "minimum": 1,
      "maximum": 5
    }
  },
  "required": ["apiKey"]
}

The apiKey property above has a "format" property. We will write a custom ajv formatter for masked properties.

It would be good to be able to limit ingestion to certain categories.

For example, in the AWS integrations you might have categories such as the following:

• [X] EC2
• [X] IAM
• [ ] RDS
• [X] Other

In order to support mechanisms such as dealing with an update operation that is reflecting an older state of source data, the integration SDK should work to ensure time stamps from the source are included in the entities/relationships.

The data model documents createdOn and updatedon for tracking the source timestamps. This should be required data, even if it is generated by the integration when it is not known/provided by the source, indicating the time that the integration collected the data, in case something else collects the data at a later time, but delivers it to the synchronizer earlier.

Once this data is ensured to be included in in the data delivered to the synchronization job, the persister should be able to rely it. It seems there is already some work it does to avoid overwriting newer data when an operation is stale.

Authorization should always be redacted, there may be others.

We should export * from https://github.com/JupiterOne/data-model/ in the SDK index.

The integration-sdk should be a monorepo with separate packages for cli, core, development, etc.

Mapped relationships are a bit more sophisticated than simple direct relationships (see https://github.com/JupiterOne/integration-sdk/blob/master/src/framework/data/createIntegrationRelationship.ts#L77). They should produce target entities if necessary and build the relationship.

cc/ @erkangz @softwarewright @philidem

This function has lofty goals, but it's current behavior isn't documented well enough, so that on a number of occasions, developers don't know what to do with source, or they don't like what it does/think it isn't clear.

It would be nice to see all of the dependsOn in a a single index.ts file instead of having to open each step file to get that information. I the metadata of the steps should be in index.ts and the step function code should be in separate files.

The gitlab integration stores an API response code in IntegrationError.code, and there may be another integration doing something similar. However, I think now that was not the best choice.

In https://github.com//JupiterOne/integration-sdk/blob/master/src/errors.ts#L7, and everywhere in the integration-sdk currently, the values for code are constant-like strings, and there are error classes that assign code so that we don't see the strings repeated where the errors are thrown (great!).

We should avoid having each integration develop their own set of codes for common things, wo that we can make some things consistent for runtime monitoring and debugging. We'll need a clear way for integrations to raise certain types of errors, and limit the use of new error codes as much as reasonable.

Consider this proposal:

/**
 * A type representing errors generated or handled by the integration runtime.
 * Integrations should not throw this error type directly. See below for
 * options.
 *
 * All errors occurring during execution of an integration should ultimately end
 * up as an `IntegrationError`, or a subclass thereof, each expressing a `code`
 * reflecting the class of the error. In case an unhandled `Error` occurs, which
 * has no `code`, then `"UNEXPECTED_ERROR"` will be used in logging the error.
 *
 * Integration code should throw these exceptions:
 *
 * ```
 *   // Thrown during `validateInvocation`, anything the user should see that
 *   // can help them fix the problem on their own.
 *   throw new IntegrationValidationError("Something that will be displayed to user so they can fix configuration")
 *
 *   // Typically thrown during `validateInvocation`. User will be informed of
 *   // what we're calling to test authentication, and the response code.
 *   throw new IntegrationProviderAuthenticationError({
 *     cause: err,
 *     endpoint: "https://something.com/api/authcheckwhatever",
 *     status?: err.status,
 *     message?: err.statusText
 *   });
 *
 *   // Thrown during resource fetching steps. The runtime will show these along
 *   // with information about what could not be obtained thanks to insufficient
 *   // access.
 *   throw new IntegrationProviderAuthorizationError({
 *     type: [], // the `_type`s that could not be obtained
 *     cause: err,
 *     endpoint: "https://something.com/api/resource",
 *     status?: err.status,
 *     message?: err.statusText
 *   });
 *
 *   // Thrown for unexpected API errors. This will be shown to users to help
 *   // them understand why some data sets may be incomplete.
 *   throw new IntegrationProviderAPIError({
 *     cause: err,
 *     endpoint: "https://something.com/api/something",
 *     status?: err.status,
 *     message?: err.statusText
 *   });
 * ```
 */
export class IntegrationError extends Error { ... }

When an integration is not allowed to read a resource endpoint, there needs to be a consistent way of reporting the event to the job log.

This would allow folks to avoid writing their own iterators that just filter the matching entities. We can do this in the SDK as-is, inefficient as it may be, but at least when we make it fast, there will be no changes in the integrations.

At the moment, the following error is thrown when a step fails to load:

TypeError: Cannot read property 'id' of undefined

This is a bit cryptic and we should try to improve the messaging around this.

The JobState object that gets created for each step can store a set of types that it has run across. After the step has completed, we can log a warning that unexpected types were detected. This information could also be stored on the IntegrationStepResult and summarized at the end.

Old:

export interface IntegrationInstance {
  ...
  config: any;
}

New:

export interface IntegrationInstance<TConfig> {
  ...
  config: TConfig;
}

In order to support visibility into execution patterns, report key events to a metrics service. This should be an interface to different implementations, at least 2, one for local execution and in a managed execution environment, a cloud-native metrics service.

Each metric should include these dimensions:

• accountId
• integrationDefinitionId
• integrationInstanceId
• integrationJobId

Let's consider starting with these metrics:

• Time to execute each step within the integration
• Total time to complete an execution
• Number of executions

We should make it standard practice to provide one end-to-end test of the integration that uses a Polly.js recording.

When an error occurs, it is very helpful to include the error code and a unique ID in the job event log for the error event. The jupiter-managed-integration-sdk would set the event description to something like, Step "Mochi" failed to complete due to error (code=Forbidden errorId=abc123-def-456-ghi-789).

Perhaps we could drop the due to error bit, since this new format will clearly indicate it was an error.

To make it easier to catch schema validation errors during development, we should generate typescript type definitions from our JSON schema for the data model.

Some integrations want to pull data since last time they ran. The previous integration provided https://bitbucket.org/lifeomic/jupiter-managed-integration-sdk/src/master/src/integration/service/lastSuccessfulSynchronizationTime.test.ts.

I think we should consider adding addEntity and addRelationship to the storage interface. This could help to avoid a pattern where folks end up creating collector variables, which just end up keeping unnecessary references around longer than needed (memory bloat in the frame). That is, I can see folks collecting all the things and calling addEntities/addRelationships just once at the end of a step, thereby negating the benefits of the flushing the store does. Of course, we should also consider adding guidance that encourages working on data in small batches.

After moving to a monorepo (#126) we should make sure the configuration for development tooling is moved to a shareable package.

Currently, we log the following when running sync command from CLI:

Synchronization job status: FINALIZE_PENDING
Entities uploaded: 120
Relationships uploaded: 148

We should add an option such as --wait that will cause the command to poll for the job to change to a terminal state.

Currently, there is a getStepStartStates function that is used to determine which steps are disabled. This is particularly useful when an integration has a set of permissions passed into the integration configuration, which are then used to conditionally disable steps. It would be great if we could couple the start state condition (IntegrationStepStartState) within each IntegrationStep instead of the getStepStartStates.ts file at the root of the src.

Old:

getStepStartStates.ts

export default function getStepStartStates(
  executionContext: IntegrationExecutionContext,
): IntegrationStepStartStates {
  return {
    'my-state': { disabled: true },
  };
}

Proposed:

const step: IntegrationStep = {
  id: '...',
  name: '...',
  types: ['...'],
  getIntegrationStepStartState(
    stepExecutionContext: IntegrationStepExecutionContext
  ): IntegrationStepStartState {
    return { disabled: true };
  },
  async executionHandler({
    instance,
    jobState,
  }: IntegrationStepExecutionContext) {
    //
  },
};

It isn't possible during pull request reviews to see if there is sensitive information in the responses. There are a couple projects doing decompression:

• https://github.com/JupiterOne/graph-azure/blob/master/test/helpers/polly.ts#L25
• https://github.com/JupiterOne/graph-snowflake/blob/master/test/index.ts#L54

Generally speaking, it would be necessary to look at the response encoding and when possible, decode and store UTF-8 text. It will be important that when the recordings are replayed, they still work.

This will allow for:

1. Capacity planning in production environments
2. Visibility into the amount of data an integration collects
3. Encourage developers to minimize utilization

Keep in mind that some metrics are auto-collected by nature of the environment, such as AWS Lambda max memory usage. Also, we may end up moving away from the simple file storage mechanism currently in use. Therefore, consider design by interface and allow for collecting the resource usage info from the storage implementation.

Perhaps JupiterOne/integration-template 😄

--module is documented here, but is not implemented.

I don't think this is a very common use case, but an entity key and a relationship key can technically be the same and we are currently using the same Set (DuplicateKeyTracker) to track the keys for each state.

IntegrationProviderAuthenticationError should be thrown when it fails to validate. That will capture more info than throw Error(). Also, remove this:

context.logger.info(
    {
      instance: context.instance,
    },
    'Validating integration config...',
  );

The runtime is already logging this information.

We support a way to indicate that one or more _type values didn't synchronize but you have to throw an error in a step to add to the list of partial _type files. We should be able to add _type value to partial data set without having to throw error

We've seen a good bit of confusion and different directory layouts, and failures only found once deployed, around the loading from a directory structure approach. This mechanism needs to be more robust, or we should consider moving to an explicit export invocationConfig from @jupiterone/graph-*/index.ts.

Along with #74 + the schema validation that goes on, this should help integration devs ensure that they aren't sending bad data prior to performing synchronization.