newrelic / newrelic-telemetry-sdk-java Goto Github PK

Recommendation: airlift gson's JsonWriter class into our codebase and attribute it appropriately.

Once we have made this public, we need to have a CLA associated with it.

The AbstractSenderBuilder in com.newrelic.telemetry use a parameterized builder, but due to the way the current code is written, defines build() to return Object.

This should be cleaned up and refactored to fix this ugliness, but without reintroducing a mass of cut-and-paste code.

There are a few cases in the sdk (and by the exporter) where having the ability to copy an Attributes instance would be helpful. We should add a method like Attributes.copy() that returns a new instance backed by a new map. Mutations to the resulting copied instance should not mutate the original instance.

The default retry and backoff behavior should be documented in the README.

In order to publish to maven central, we have to make sure the poms contain:

• project name
• project description
• project url
• license information
• scm url
• developer information

This is the message we got when trying to close the sonatype staging thing:

Invalid POM: /com/newrelic/telemetry/telemetry-components/0.2.0/telemetry-components-0.2.0.pom: Project name missing, Project description missing, Project URL missing, License information missing, SCM URL missing, Developer information missing

1. Make sure that INFO is pretty quiet, post-startup
2. Review log levels for various response retries
3. ... review spec and figure out where else we're out of compliance.

There is an internal doc that suggests we change the license header in all source from:

/*
 * ---------------------------------------------------------------------------------------------
 *  Copyright (c) 2019 New Relic Corporation. All rights reserved.
 *  Licensed under the Apache 2.0 License. See LICENSE in the project root directory for license information.
 * --------------------------------------------------------------------------------------------
 */

to this new format:

/*
 * Copyright 2019 New Relic Corporation. All rights reserved.
 * SPDX-License-Identifier: Apache-2.0
 */

We should just do this all in one go.

Possibilities:

Have the okhttp & java11 modules depend on the telemetry module, with a factory class that builds the telemetry client in one method call.

Audit logging would be a configuration option, off by default, that, when enabled would log json payloads sent to the API endpoints, at DEBUG level.

Include very clear documentation that if the payloads contain sensitive information that that information would be sent to wherever logs are configured to go.

We also probably want to change the default on the SimpleMetricBatchSender to not be an infinite timeout, while we're at it.

Also, if the current behavior with timeouts is sub-optimal, let's make it better.

The MetricBatchJsonGenerator should be refactored so that it is less dependent on metrics alone. At the end of this refactoring, we should have a class that can marshal any telemetry type (of which there are just metrics right now) to json.

When marshaling metrics, the resulting json should still confirm to the current format.

See documentation:
https://docs.newrelic.com/docs/new-relic-trace-api-early-access-guide

For this enhancement, we should do the following:

• Define the java-side data model
• Build a TraceSender that has a similar function to MetricBatchSender

This issue does not require sample code or documentation (yet).

The backend API supports sending multiple batches in one payload. We should support that in the SDK.

(Migrate to Jira)

See here for details: https://github.com/newrelic/newrelic-telemetry-sdk-specs/blob/master/communication.md#graceful-degradation

We should handle connect/response timeouts from the metric/trace APIs more gracefully, perhaps by doing a single retry of the send before giving up.

service.name is a very useful, and almost-always-desired attribute for both metrics and spans. We should make it easy for users to set the service.name attribute on metric batches.

This could be a first-class field on the MetricBatchSender, the MetricBatch itself, or somehow on the metrics. A developer-friendly API should be the first consideration on figuring out the right way to implement this.

For example, if you set service.name as a attribute on a span, but don't explicitly set the serviceName field via the SpanBuilder, then the resulting service.name attribute is not written at all.

It should be version 0.2.1

create a release in GH
make sure to update the release version to 0.3.0-SNAPSHOT after the release is complete

When they are available, we should link to them.

Take the manual process and automate it.

The manual process will be documented <Xia has notes, convert to doc. insert link to google doc>

We have managed to create Opentelemetry Spans without much issue and through debugging, it looks like the spans are using New Relic format.
We have tested the API key and the URI successfully with the curl command: https://docs.newrelic.com/docs/understand-dependencies/distributed-tracing/trace-api/report-new-relic-format-traces-trace-api
We are not receiving any errors whether or not we use the URIoverride. We have forced errors by using the wrong keys. Is there a way to troubleshoot this?

Create all necessary Javadocs for Span related sources

See https://github.com/newrelic/newrelic-telemetry-sdk-specs/blob/master/communication.md#user-agent

From looking at user-agent metadata, it looks as though the SDK is being pulled into a fat jar, and we're reading the manifest of the fat jar, rather than the original SDK jar.

We're ending up with user-agent strings that look like: "NewRelic-Java-TelemetrySDK/20.6.7029", which is definitely not a version that we've released.

This issue will also apply to all of our java-based exporters that are reading versions from the jar manifest.

The analogue of the SimpleMetricBatchSender

Similar to the existing examples, an example that reports traces should be created (perhaps com.newrelic.telemetry.TracesExample.

This should also include a blurb in the README.md for the telemetry_examples and some clear description/instructions/documentation.

1. Add method to be able to send data without retry logic, returning the Response object to the user with no validation.
2. Add a method to be able to send data without retry logic, but throw the appropriate ResponseException.
3. Consider making the creation of the ResponseException, given a Response, a public API.

If you set an the attribute error to true in a span, then the DT UI knows it was an span with an error in it. we should provide that as a first-class option in our Span builder.

#124 implemented the specified backoff strategy, but used a set of reasonable defaults. These defaults may not be applicable in every user environment, and so they should be made configurable.

See this for the spec:
https://github.com/newrelic/newrelic-telemetry-sdk-specs/blob/master/communication.md#dropping-data

We have one for developers, but not for users.

We need to verify that the default behavior of our TelemetryClient with the OkHttpPoster. Specifically, we should look to confirm that connections are held open (keep alives) to prevent recurring connection setup overhead.

The version should be obtained from the package of the BatchDataSender inside the jar. We should confirm that this value is being fetched correctly and put into the user agent when using the jar (and not just running from the IDE).

it just calls itself. 🤦‍♂

Our one-stop-shop for telemetry should now include spans!

Some users might expect the examples jar to be runnable (eg. it has a main class defined in the manifest). The use case is that a new user clones the repo, looks at some docs, reads some code, and then wants to try it out. They build the jar with gradle and then try to run it via java -jar <jar> <insights_key>.

Right now this doesn't work. We should make that better, and include docs.

Also, update the documentation and the examples to account for this.

At present, most of the functionality is within a module called metrics. There is non-metric-specific code in this module (HttpPoster and Attributes, for example). So, in order to better accommodate support for Traces, we should do some project restructuring and decide how to proceed (not yet sure which would be best).

Idea 1: Rename metrics to telemetry-core

Pros: Simple change, no code has to move. Traces can sit next to Metrics in the same source module.

Cons: Docs/deps change. Published artifact names change.

Idea 2: Add modules telemetry-core and traces

Common/core code that can be shared between telemetry types would be moved into this new "core" module. Establishes a pattern in which each telemetry type has its own module, and these modules depend on the core,

Pros: Simple. Metrics artifact doesn't need to change.

Cons: Increased number of modules. Increased dependency tree complexity.

Idea 3: Put Traces into the metrics module

Pros: Least amount of work

Cons: Naming will be wrong. Starts a pattern which will be harder to break when other telemetry types come into play.

When we're dropping metrics due to invalid values, it would be good to log (at DEBUG?) the number of metrics that we're dropping.

Details needed on how this might be enabled.

The general idea is that there should be an easy way to keep the SDK in place, but simply disable any actual sending of data with a single (configuration?) change.

Details are here: https://source.datanerd.us/agents/telemetry-sdk-specs/blob/master/configuration-and-logging.md#no-op-behavior

We don't yet have the ability to generate a json payload for the traces (spans) ingest API. We should make a data model and appropriate marshalers that can create this JSON.