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

Add opentracing integration #860

Merged
merged 7 commits into from
Jun 2, 2020

Conversation

austenLacy
Copy link
Contributor

@austenLacy austenLacy commented May 28, 2020

These changes allow for a rollbar.js user to inject an OpenTracing tracer object as a configuration option.

Whenever an error or message is logged via Rollbar we will check if the client has as tracer set. If that tracer is valid then we set the new occurrence UUID as metadata on the opentracing span as well as add the opentracing trace ID and span ID to the occurrence as custom metadata.

Implementation Examples


automatic instrumentation

const tracer = require('ls-trace').init({
  experimental: { b3: true }
});

const Rollbar = require('rollbar');
const rollbar = new Rollbar({
  accessToken: process.env.ROLLBAR_ACCESS_TOKEN,
  captureUncaught: true,
  captureUnhandledRejections: true,
  tracer: tracer // Add tracer here...
});

manual instrumentation

app.get('/hello', function getHello(req, res) {
  let span = null;

  try {
    const name = req.query.name;
    const parentSpan = tracer.scope().active();
    span = tracer.startSpan('getHelloWorld', { childOf: parentSpan });

    if (name && name.toLowerCase() === 'error') {
      throw new Error("Can't say hello to an error!");
    }

    res.status(200).send({
      message: `Hello ${name ? name : 'world'}!`
    });

    span.finish();
  } catch (err) {
    rollbar.error(err, req, { opentracing_span_id: span.context().toSpanId(), opentracing_trace_id: span.context().toTraceId() }, (internalRbErr, rbRes) => {
      if (internalRbErr) {
        console.error(internalRbErr);
      } else {
        if (span) {
          span.setTag('rollbar_uuid', rbRes.uuid);
          span.setTag('has_rollbar_error', true);
          span.finish();
        }
      }
    });

    res.status(500).send({ error: err.message });
  }
});

@austenLacy austenLacy requested a review from waltjones May 28, 2020 15:16
…e span when validating tracer at client instantiation
src/browser/rollbar.js Outdated Show resolved Hide resolved
Copy link
Contributor

@waltjones waltjones left a comment

Choose a reason for hiding this comment

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

Looks good. 👍

@waltjones
Copy link
Contributor

Changes look good. 👍

@austenLacy austenLacy merged commit 84bbcd7 into master Jun 2, 2020
@@ -13,7 +13,8 @@ function baseData(item, options, callback) {
language: 'javascript',
framework: item.framework || options.framework,
uuid: item.uuid,
notifier: JSON.parse(JSON.stringify(options.notifier))
notifier: JSON.parse(JSON.stringify(options.notifier)),
custom: item.custom

Choose a reason for hiding this comment

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

This results in duplicated properties in Occurrences view... Could we revert this part?

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 the intent is to support the custom key as defined by the API. https://explorer.docs.rollbar.com/#operation/create-item

Maybe a config option for this is the best way to meet both needs.

mudetroit pushed a commit that referenced this pull request Mar 14, 2024
* add lightstep integration

* separate validation of tracer + span b/c we dont always have an active span when validating tracer at client instantiation

* add item.custom data to custom field in browserjs implementation

* move tracer config to Client rather than each implementation

* revert spreading of data.custom attrs to root data obj in browserjs

* add item.data.custom to all occurrences for both nodejs and browserjs

* add item.data.custom for react-native too
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.

4 participants