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

Added EU endpoint support and Unit tests in SenderConfiguration and the BatchSenders for all data types. #276

Merged
merged 9 commits into from
Jun 28, 2021

Conversation

yamnihcg
Copy link
Contributor

@yamnihcg yamnihcg commented Jun 11, 2021

The following PR adds the .setRegion() method to the SenderConfigurationBuilder API. This method allows you to define a region (US or EU) so that the appropriate production endpoint (corresponding to that region) is used. The default endpoint is the US production endpoint.

If you want to use an endpoint other than the US / EU production endpoints, use the existing .endpoint() method in the SenderConfigurationBuilder API.

Below are some examples on how to use the new API. In these examples, we use the MetricBatchSender / MetricBatchSenderFactory.

US Production

String licenseKey = args[0];
MetricBatchSenderFactory factory = MetricBatchSenderFactory.fromHttpImplementation(OkHttpPoster::new);
MetricBatchSender sender = MetricBatchSender.create(factory.configureWith(licenseKey).useLicenseKey(true).build());

Note: Since the default endpoint is the US production endpoint, we did not use .setRegion() in this example. Also, a US license key is needed to send data to this endpoint.

EU Production

String licenseKey = args[0];
MetricBatchSenderFactory factory = MetricBatchSenderFactory.fromHttpImplementation(OkHttpPoster::new);
MetricBatchSender sender = MetricBatchSender.create(factory.configureWith(licenseKey).useLicenseKey(true).setRegion("EU").build());

Note: An EU license key is needed to send data to this endpoint.

Staging / Setting Your Own URL

URL endpointURL = new URL("http://localhost:1439/v1/accounts/events");
String licenseKey = args[0];
MetricBatchSenderFactory factory = MetricBatchSenderFactory.fromHttpImplementation(OkHttpPoster::new);
MetricBatchSender sender =
    MetricBatchSender.create(factory.configureWith(licenseKey).useLicenseKey(true).endpoint(endpointURL).build());

@yamnihcg yamnihcg requested a review from XiXiaPdx June 11, 2021 22:09
@yamnihcg yamnihcg changed the title Added EU endpoint support in SenderConfiguration and the BatchSenders for Metric, Log, and Trace data types. Added EU endpoint support in SenderConfiguration and the BatchSenders for all data types. Jun 21, 2021
@yamnihcg yamnihcg changed the title Added EU endpoint support in SenderConfiguration and the BatchSenders for all data types. Added EU endpoint support and Tests in SenderConfiguration and the BatchSenders for all data types. Jun 21, 2021
@yamnihcg yamnihcg changed the title Added EU endpoint support and Tests in SenderConfiguration and the BatchSenders for all data types. Added EU endpoint support and Unit tests in SenderConfiguration and the BatchSenders for all data types. Jun 21, 2021
Copy link
Contributor

@XiXiaPdx XiXiaPdx left a comment

Choose a reason for hiding this comment

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

Looking great! A few more tests, now that user can override region with endpoint

return endpointRegion;
}

public boolean isUserProvideEndpoint() {
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick spelling - isUserProvidedEnpoint()

}
} else if (userRegion.equals("EU")) {
try {
url = new URL(EUROPEAN_URL + METRICS_PATH);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make the creation of the Default URL be simple and consistent with the creation of the European Url?

*
* @param endpoint A full {@link URL}, including the path.
* @return this builder.
*/
public SenderConfigurationBuilder endpoint(URL endpoint) {
SenderConfiguration.userProvidedEndpoint = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

please provide a test that userProvidedEndpoint will be set to true

boolean isCustomEndpoint = configuration.isUserProvideEndpoint();

URL url = null;
if (isCustomEndpoint == true) {
Copy link
Contributor

Choose a reason for hiding this comment

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

please add a test in each BatchSender to confirm custom endpoint will override region

Copy link
Contributor

@XiXiaPdx XiXiaPdx left a comment

Choose a reason for hiding this comment

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

just a few items left. Thank you!

* @return this builder
*/
public SenderConfigurationBuilder setRegion(String region) throws IllegalArgumentException {
// Add IllegalArgumentException if region isn't US or EU
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this comment plz

}

@Test
void defaultEndpointAsStringTest() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this test just confirms toString() works? If. so, this test can be removed.

}

@Test
void userEndpointAsStringTest() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

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

same for this test, I think it can be removed

@yamnihcg
Copy link
Contributor Author

EventBatchSender:

Default (to US Prod, US License Key): Successful
Set Region (to US Prod, US License Key): Successful
Set Region (to EU Prod, EU License Key): Successful
User Provided Endpoint: Successful

MetricBatchSender:

Default (to US Prod, US License Key): Successful
Set Region (to US Prod, US License Key): Successful
Set Region (to EU Prod, EU License Key): Successful
User Provided Endpoint: Successful

LogBatchSender:

Default (to US Prod, US License Key): Successful
Set Region (to US Prod, US License Key): Successful
Set Region (to EU Prod, EU License Key): Successful
User Provided Endpoint: Successful

SpanBatchSender:

Default (to US Prod, US License Key): Successful
Set Region (to US Prod, US License Key): Successful
Set Region (to EU Prod, EU License Key): Successful
User Provided Endpoint: Successful

@XiXiaPdx
Copy link
Contributor

This looks great @yamnihcg and thank you for persistently chipping away at this! 🚀 🚀

@yamnihcg yamnihcg merged commit e848562 into main Jun 28, 2021
@yamnihcg yamnihcg deleted the eu-endpoint-feature-branch branch June 28, 2021 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants