Commit ba1e9892 authored by Butkovits Atila's avatar Butkovits Atila
Browse files

Backed out changeset bbea0c20d7fe (bug 1732724) for causing Xpcshell failures...

Backed out changeset bbea0c20d7fe (bug 1732724) for causing Xpcshell failures at test_targeting.js. CLOSED TREE
parent e8ced8b5
Loading
Loading
Loading
Loading
+7 −14
Original line number Diff line number Diff line
@@ -1068,28 +1068,21 @@ Unlike other Activity Stream pings, this is a Firefox Events telemetry event, an
### Experiment attribute errors

This records whether issues were encountered with any of the targeting attributes used in the experiment enrollment or message targeting.
Two different types of events are sent: `attribute_error` and `attribute_timeout` along with the attribute that caused it. An attribute
is a variable inside the JEXL targeting expression that is evaluated client side by the browser.
Two different types of events are sent: `attribute_error` and `attribute_timeout` along with the attribute that caused it.

```js
{
[
  "messaging_experiments",
  "targeting",
  "attribute_error", // event
  "foo", // attribute,
  "extra_keys": {
    "source": "message id or experiment slug",
  },
},
{
  "foo" // attribute
],
[
  "messaging_experiments",
  "targeting",
  "attribute_timeout", // event
  "bar", // attribute,
  "extra_keys": {
    "source": "message id or experiment slug",
  },
}
  "bar" // attribute
]
```

## Firefox Onboarding (about:welcome) pings
+0 −3
Original line number Diff line number Diff line
@@ -743,9 +743,6 @@ this.ASRouterTargeting = {
          return result.value;
        }
      }
      // Used to report the source of the targeting error in the case of
      // undesired events
      targetingContext.setTelemetrySource(message.id);
      result = await targetingContext.evalWithDefault(message.targeting);
      if (shouldCache) {
        jexlEvaluationCache.set(message.targeting, {
+0 −22
Original line number Diff line number Diff line
@@ -313,7 +313,6 @@ describe("ASRouterTargeting", () => {
    fakeTargetingContext = {
      combineContexts: sandbox.stub(),
      evalWithDefault: sandbox.stub().resolves(),
      setTelemetrySource: sandbox.stub(),
    };
    globals = new GlobalOverrider();
    globals.set(
@@ -323,10 +322,6 @@ describe("ASRouterTargeting", () => {
          return fakeTargetingContext.combineContexts.apply(sandbox, args);
        }

        setTelemetrySource(id) {
          fakeTargetingContext.setTelemetrySource(id);
        }

        evalWithDefault(expr) {
          return fakeTargetingContext.evalWithDefault(expr);
        }
@@ -339,23 +334,6 @@ describe("ASRouterTargeting", () => {
    sandbox.restore();
    globals.restore();
  });
  it("should provide message.id as source", async () => {
    await ASRouterTargeting.checkMessageTargeting(
      {
        id: "message",
        targeting: "true",
      },
      fakeTargetingContext,
      sandbox.stub(),
      false
    );
    assert.calledOnce(fakeTargetingContext.evalWithDefault);
    assert.calledWithExactly(fakeTargetingContext.evalWithDefault, "true");
    assert.calledWithExactly(
      fakeTargetingContext.setTelemetrySource,
      "message"
    );
  });
  it("should cache evaluation result", async () => {
    evalStub.resolves(true);
    let targetingContext = new global.TargetingContext();
+8 −29
Original line number Diff line number Diff line
@@ -67,9 +67,7 @@ const TargetingEnvironment = {
};

class TargetingContext {
  #telemetrySource = null;

  constructor(customContext, options = { source: null }) {
  constructor(customContext) {
    if (customContext) {
      this.ctx = new Proxy(customContext, {
        get: (customCtx, prop) => {
@@ -83,29 +81,11 @@ class TargetingContext {
      this.ctx = TargetingEnvironment;
    }

    // Used in telemetry to report where the targeting expression is coming from
    this.#telemetrySource = options.source;

    // Enable event recording
    Services.telemetry.setEventRecordingEnabled(TARGETING_EVENT_CATEGORY, true);
  }

  setTelemetrySource(source) {
    if (source) {
      this.#telemetrySource = source;
    }
  }

  _sendUndesiredEvent(eventData) {
    if (this.#telemetrySource) {
      Services.telemetry.recordEvent(
        TARGETING_EVENT_CATEGORY,
        TARGETING_EVENT_METHOD,
        eventData.event,
        eventData.value,
        { source: this.#telemetrySource }
      );
    } else {
    Services.telemetry.recordEvent(
      TARGETING_EVENT_CATEGORY,
      TARGETING_EVENT_METHOD,
@@ -113,7 +93,6 @@ class TargetingContext {
      eventData.value
    );
  }
  }

  /**
   * Wrap each property of context[key] with a Proxy that captures errors and
@@ -236,7 +215,7 @@ class TargetingContext {
   * @param {string} expression JEXL expression
   * @returns {promise} Evaluation result
   */
  evalWithDefault(expression, options = { source: null }) {
  evalWithDefault(expression) {
    return FilterExpressions.eval(
      expression,
      this.createContextWithTimeout(this.ctx)
+0 −54
Original line number Diff line number Diff line
@@ -260,57 +260,3 @@ add_task(async function test_targeting_os() {
  );
  Assert.ok(res, `Should detect platform version got: ${res}`);
});

add_task(async function test_targeting_source_constructor() {
  Services.telemetry.clearEvents();
  const targeting = new TargetingContext(
    { ctx: { foo: true } },
    { source: "unit_testing" }
  );

  let res = await targeting.eval("ctx.foo");
  Assert.ok(res, "Should eval to true");

  let expectedEvents = [
    [
      "messaging_experiments",
      "targeting",
      "attribute_error",
      "bar.foo",
      JSON.stringify({ source: "unit_testing" }),
    ],
  ];
  try {
    await targeting.eval("bar.foo");
  } catch (e) {}

  TelemetryTestUtils.assertEvents(expectedEvents);
  Services.telemetry.clearEvents();
});

add_task(async function test_targeting_source_override() {
  Services.telemetry.clearEvents();
  const targeting = new TargetingContext(
    { ctx: { foo: true } },
    { source: "unit_testing" }
  );

  let res = await targeting.eval("ctx.foo");
  Assert.ok(res, "Should eval to true");

  let expectedEvents = [
    [
      "messaging_experiments",
      "targeting",
      "attribute_error",
      "bar.foo",
      JSON.stringify({ source: "override" }),
    ],
  ];
  try {
    await targeting.evalWithDefault("bar.foo", { source: "override" });
  } catch (e) {}

  TelemetryTestUtils.assertEvents(expectedEvents);
  Services.telemetry.clearEvents();
});
Loading