You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@phoenix.apache.org by jyates <gi...@git.apache.org> on 2014/07/23 00:31:25 UTC

[GitHub] phoenix pull request: Implement HTrace based tracing

GitHub user jyates opened a pull request:

    https://github.com/apache/phoenix/pull/5

    Implement HTrace based tracing

    Small issue in that not everyone serializes htrace annotations, but that's an
    open question of the right way to do that anyways
    
    Adding tracing to:
     - MutationState
     - query plan tracing
     - iterators
    
    Metrics writing is generalized to support eventual hadoop1 implementation.
    Also, supporting test-skipping with a custom Hadoop1 test runner + annotation
    
    Default builds to hadoop2, rather than hadoop1 (particularly as hadoop1 is
    now a second-class citizen).

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jyates/phoenix tracing

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/phoenix/pull/5.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #5
    
----
commit 59b1ad2e6cb90e4906ca29bd1964617b2ac7f6c9
Author: Jesse Yates <jy...@apache.org>
Date:   2014-06-06T23:11:32Z

    Implement HTrace based tracing
    
    Small issue in that not everyone serializes htrace annotations, but that's an
    open question of the right way to do that anyways
    
    Adding tracing to:
     - MutationState
     - query plan tracing
     - iterators
    
    Metrics writing is generalized to support eventual hadoop1 implementation.
    Also, supporting test-skipping with a custom Hadoop1 test runner + annotation
    
    Default builds to hadoop2, rather than hadoop1 (particularly as hadoop1 is
    now a second-class citizen).

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: Implement HTrace based tracing

Posted by jyates <gi...@git.apache.org>.
Github user jyates commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/5#discussion_r15407718
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/trace/PhoenixTableMetricsWriter.java ---
    @@ -0,0 +1,255 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.phoenix.trace;
    +
    +import static org.apache.phoenix.metrics.MetricInfo.ANNOTATION;
    +import static org.apache.phoenix.metrics.MetricInfo.TAG;
    +import static org.apache.phoenix.metrics.MetricInfo.DESCRIPTION;
    +import static org.apache.phoenix.metrics.MetricInfo.END;
    +import static org.apache.phoenix.metrics.MetricInfo.HOSTNAME;
    +import static org.apache.phoenix.metrics.MetricInfo.PARENT;
    +import static org.apache.phoenix.metrics.MetricInfo.SPAN;
    +import static org.apache.phoenix.metrics.MetricInfo.START;
    +import static org.apache.phoenix.metrics.MetricInfo.TRACE;
    +
    +import java.sql.Connection;
    +import java.sql.PreparedStatement;
    +import java.sql.SQLException;
    +import java.util.ArrayList;
    +import java.util.List;
    +
    +import org.apache.commons.logging.Log;
    +import org.apache.commons.logging.LogFactory;
    +import org.apache.hadoop.conf.Configuration;
    +import org.apache.hadoop.hbase.HBaseConfiguration;
    +import org.apache.phoenix.metrics.MetricInfo;
    +import org.apache.phoenix.metrics.MetricsWriter;
    +import org.apache.phoenix.metrics.PhoenixAbstractMetric;
    +import org.apache.phoenix.metrics.PhoenixMetricTag;
    +import org.apache.phoenix.metrics.PhoenixMetricsRecord;
    +import org.apache.phoenix.util.QueryUtil;
    +
    +import com.google.common.annotations.VisibleForTesting;
    +import com.google.common.base.Joiner;
    +
    +/**
    + * Sink that writes phoenix metrics to a phoenix table
    + * <p>
    + * Each metric record should only correspond to a single completed span. Each span is only updated
    + * in the phoenix table <i>once</i>
    + */
    +public class PhoenixTableMetricsWriter implements MetricsWriter {
    +
    +    private static final String VARIABLE_VALUE = "?";
    +
    +    public static final Log LOG = LogFactory.getLog(PhoenixTableMetricsWriter.class);
    +
    +    private static final Joiner COLUMN_JOIN = Joiner.on(".");
    +    static final String TAG_FAMILY = "tags";
    +    /** Count of the number of tags we are storing for this row */
    +    static final String TAG_COUNT = COLUMN_JOIN.join(TAG_FAMILY, "count");
    +
    +    static final String ANNOTATION_FAMILY = "annotations";
    +    static final String ANNOTATION_COUNT = COLUMN_JOIN.join(ANNOTATION_FAMILY, "count");
    +
    +    /** Join strings on a comma */
    +    private static final Joiner COMMAS = Joiner.on(',');
    +
    +    private Connection conn;
    +
    +    private String table;
    +
    +    @Override
    +    public void initialize() {
    +        try {
    +            // create the phoenix connection
    +            Configuration conf = HBaseConfiguration.create();
    +            Connection conn = QueryUtil.getConnection(conf);
    +            // enable bulk loading when we have enough data
    +            conn.setAutoCommit(true);
    +
    +            String tableName =
    +                    conf.get(TracingCompat.TARGET_TABLE_CONF_KEY,
    +                        TracingCompat.DEFAULT_STATS_TABLE_NAME);
    +
    +            initializeInternal(conn, tableName);
    +        } catch (Exception e) {
    +            throw new RuntimeException(e);
    +        }
    +    }
    +
    +    private void initializeInternal(Connection conn, String tableName) throws SQLException {
    +        this.conn = conn;
    +
    +        // ensure that the target table already exists
    +        createTable(conn, tableName);
    +    }
    +
    +    /**
    +     * Used for <b>TESTING ONLY</b>
    +     * <p>
    +     * Initialize the connection and setup the table to use the
    +     * {@link TracingCompat#DEFAULT_STATS_TABLE_NAME}
    +     * @param conn to store for upserts and to create the table (if necessary)
    +     * @throws SQLException if any phoenix operation fails
    +     */
    +    @VisibleForTesting
    +    public void initForTesting(Connection conn) throws SQLException {
    +        initializeInternal(conn, TracingCompat.DEFAULT_STATS_TABLE_NAME);
    +    }
    +
    +    /**
    +     * Create a stats table with the given name. Stores the name for use later when creating upsert
    +     * statements
    +     * @param conn connection to use when creating the table
    +     * @param table name of the table to create
    +     * @throws SQLException if any phoenix operations fails
    +     */
    +    private void createTable(Connection conn, String table) throws SQLException {
    +        // only primary-key columns can be marked non-null
    +        String ddl =
    +                "create table if not exists " + table + "( " + 
    +                        TRACE.columnName + " bigint not null, " +
    +                        PARENT.columnName + " bigint not null, " +
    +                        SPAN.columnName + " bigint not null, " +
    +                        DESCRIPTION.columnName + " varchar, " +
    +                        START.columnName + " bigint, " +
    +                        END.columnName + " bigint, " +
    +                        HOSTNAME.columnName + " varchar, " +
    +                        TAG_COUNT + " smallint, " +
    +                        ANNOTATION_COUNT + " smallint" +
    +                        "  CONSTRAINT pk PRIMARY KEY (" + TRACE.columnName + ", "
    +                            + PARENT.columnName + ", " + SPAN.columnName + "))\n";
    +        PreparedStatement stmt = conn.prepareStatement(ddl);
    +        stmt.execute();
    +        this.table = table;
    +    }
    +
    +    @Override
    +    public void flush() {
    +        try {
    +            this.conn.commit();
    +            this.conn.rollback();
    +        } catch (SQLException e) {
    +            LOG.error("Failed to commit changes to table", e);
    +        }
    +    }
    +
    +    /**
    +     * Add a new metric record to be written.
    +     * @param record
    +     */
    +    @Override
    +    public void addMetrics(PhoenixMetricsRecord record) {
    +        // its not a tracing record, we are done. This could also be handled by filters, but safer
    +        // to do it here, in case it gets misconfigured
    +        if (!record.name().startsWith(TracingCompat.METRIC_SOURCE_KEY)) {
    +            return;
    +        }
    +        String stmt = "UPSERT INTO " + table + " (";
    --- End diff --
    
    Considered that, but the number of tags and annotations are variable, so you can't just build a single statement - you would have to build a separate one for the tags/annotations each time, so you might as well just do them all at once


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: Implement HTrace based tracing

Posted by JamesRTaylor <gi...@git.apache.org>.
Github user JamesRTaylor commented on the pull request:

    https://github.com/apache/phoenix/pull/5#issuecomment-50102170
  
    The code looks very clean, @jyates. Would love to get a demo and understand the overall flow and any limitations a bit better. What's the typical way a Phoenix user will interact with this to get their metrics? Got any time tomorrow?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: Implement HTrace based tracing

Posted by JamesRTaylor <gi...@git.apache.org>.
Github user JamesRTaylor commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/5#discussion_r15383971
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/trace/PhoenixTableMetricsWriter.java ---
    @@ -0,0 +1,255 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.phoenix.trace;
    +
    +import static org.apache.phoenix.metrics.MetricInfo.ANNOTATION;
    +import static org.apache.phoenix.metrics.MetricInfo.TAG;
    +import static org.apache.phoenix.metrics.MetricInfo.DESCRIPTION;
    +import static org.apache.phoenix.metrics.MetricInfo.END;
    +import static org.apache.phoenix.metrics.MetricInfo.HOSTNAME;
    +import static org.apache.phoenix.metrics.MetricInfo.PARENT;
    +import static org.apache.phoenix.metrics.MetricInfo.SPAN;
    +import static org.apache.phoenix.metrics.MetricInfo.START;
    +import static org.apache.phoenix.metrics.MetricInfo.TRACE;
    +
    +import java.sql.Connection;
    +import java.sql.PreparedStatement;
    +import java.sql.SQLException;
    +import java.util.ArrayList;
    +import java.util.List;
    +
    +import org.apache.commons.logging.Log;
    +import org.apache.commons.logging.LogFactory;
    +import org.apache.hadoop.conf.Configuration;
    +import org.apache.hadoop.hbase.HBaseConfiguration;
    +import org.apache.phoenix.metrics.MetricInfo;
    +import org.apache.phoenix.metrics.MetricsWriter;
    +import org.apache.phoenix.metrics.PhoenixAbstractMetric;
    +import org.apache.phoenix.metrics.PhoenixMetricTag;
    +import org.apache.phoenix.metrics.PhoenixMetricsRecord;
    +import org.apache.phoenix.util.QueryUtil;
    +
    +import com.google.common.annotations.VisibleForTesting;
    +import com.google.common.base.Joiner;
    +
    +/**
    + * Sink that writes phoenix metrics to a phoenix table
    + * <p>
    + * Each metric record should only correspond to a single completed span. Each span is only updated
    + * in the phoenix table <i>once</i>
    + */
    +public class PhoenixTableMetricsWriter implements MetricsWriter {
    +
    +    private static final String VARIABLE_VALUE = "?";
    +
    +    public static final Log LOG = LogFactory.getLog(PhoenixTableMetricsWriter.class);
    +
    +    private static final Joiner COLUMN_JOIN = Joiner.on(".");
    +    static final String TAG_FAMILY = "tags";
    +    /** Count of the number of tags we are storing for this row */
    +    static final String TAG_COUNT = COLUMN_JOIN.join(TAG_FAMILY, "count");
    +
    +    static final String ANNOTATION_FAMILY = "annotations";
    +    static final String ANNOTATION_COUNT = COLUMN_JOIN.join(ANNOTATION_FAMILY, "count");
    +
    +    /** Join strings on a comma */
    +    private static final Joiner COMMAS = Joiner.on(',');
    +
    +    private Connection conn;
    +
    +    private String table;
    +
    +    @Override
    +    public void initialize() {
    +        try {
    +            // create the phoenix connection
    +            Configuration conf = HBaseConfiguration.create();
    +            Connection conn = QueryUtil.getConnection(conf);
    +            // enable bulk loading when we have enough data
    +            conn.setAutoCommit(true);
    +
    +            String tableName =
    +                    conf.get(TracingCompat.TARGET_TABLE_CONF_KEY,
    +                        TracingCompat.DEFAULT_STATS_TABLE_NAME);
    +
    +            initializeInternal(conn, tableName);
    +        } catch (Exception e) {
    +            throw new RuntimeException(e);
    +        }
    +    }
    +
    +    private void initializeInternal(Connection conn, String tableName) throws SQLException {
    +        this.conn = conn;
    +
    +        // ensure that the target table already exists
    +        createTable(conn, tableName);
    +    }
    +
    +    /**
    +     * Used for <b>TESTING ONLY</b>
    +     * <p>
    +     * Initialize the connection and setup the table to use the
    +     * {@link TracingCompat#DEFAULT_STATS_TABLE_NAME}
    +     * @param conn to store for upserts and to create the table (if necessary)
    +     * @throws SQLException if any phoenix operation fails
    +     */
    +    @VisibleForTesting
    +    public void initForTesting(Connection conn) throws SQLException {
    +        initializeInternal(conn, TracingCompat.DEFAULT_STATS_TABLE_NAME);
    +    }
    +
    +    /**
    +     * Create a stats table with the given name. Stores the name for use later when creating upsert
    +     * statements
    +     * @param conn connection to use when creating the table
    +     * @param table name of the table to create
    +     * @throws SQLException if any phoenix operations fails
    +     */
    +    private void createTable(Connection conn, String table) throws SQLException {
    +        // only primary-key columns can be marked non-null
    +        String ddl =
    +                "create table if not exists " + table + "( " + 
    +                        TRACE.columnName + " bigint not null, " +
    +                        PARENT.columnName + " bigint not null, " +
    +                        SPAN.columnName + " bigint not null, " +
    +                        DESCRIPTION.columnName + " varchar, " +
    +                        START.columnName + " bigint, " +
    +                        END.columnName + " bigint, " +
    +                        HOSTNAME.columnName + " varchar, " +
    +                        TAG_COUNT + " smallint, " +
    +                        ANNOTATION_COUNT + " smallint" +
    +                        "  CONSTRAINT pk PRIMARY KEY (" + TRACE.columnName + ", "
    +                            + PARENT.columnName + ", " + SPAN.columnName + "))\n";
    +        PreparedStatement stmt = conn.prepareStatement(ddl);
    +        stmt.execute();
    +        this.table = table;
    +    }
    +
    +    @Override
    +    public void flush() {
    +        try {
    +            this.conn.commit();
    +            this.conn.rollback();
    +        } catch (SQLException e) {
    +            LOG.error("Failed to commit changes to table", e);
    +        }
    +    }
    +
    +    /**
    +     * Add a new metric record to be written.
    +     * @param record
    +     */
    +    @Override
    +    public void addMetrics(PhoenixMetricsRecord record) {
    +        // its not a tracing record, we are done. This could also be handled by filters, but safer
    +        // to do it here, in case it gets misconfigured
    +        if (!record.name().startsWith(TracingCompat.METRIC_SOURCE_KEY)) {
    +            return;
    +        }
    +        String stmt = "UPSERT INTO " + table + " (";
    --- End diff --
    
    Ideally, if possible, you'd want to build this UPSERT statement once, call conn.prepareStatement(upsert) on it, and then here just bind the bind variables and execute it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: Implement HTrace based tracing

Posted by jyates <gi...@git.apache.org>.
Github user jyates commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/5#discussion_r15407823
  
    --- Diff: phoenix-core/src/it/java/org/apache/phoenix/trace/DisableableMetricsWriter.java ---
    @@ -0,0 +1,83 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.phoenix.trace;
    +
    +import java.sql.SQLException;
    +import java.util.concurrent.atomic.AtomicBoolean;
    +
    +import org.apache.commons.logging.Log;
    +import org.apache.commons.logging.LogFactory;
    +import org.apache.phoenix.metrics.MetricsWriter;
    +import org.apache.phoenix.metrics.PhoenixMetricsRecord;
    +
    +/**
    + *
    + */
    +public class DisableableMetricsWriter implements MetricsWriter {
    +
    +    private static final Log LOG = LogFactory.getLog(DisableableMetricsWriter.class);
    +    private PhoenixTableMetricsWriter writer;
    --- End diff --
    
    Its just a test class, no too worried about that kind of stuff. Not assumed to be thread safe, though it probably is.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: Implement HTrace based tracing

Posted by JamesRTaylor <gi...@git.apache.org>.
Github user JamesRTaylor commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/5#discussion_r15384076
  
    --- Diff: phoenix-core/src/it/java/org/apache/phoenix/trace/PhoenixTracingEndToEndIT.java ---
    @@ -0,0 +1,401 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.phoenix.trace;
    +
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.assertTrue;
    +
    +import java.sql.Connection;
    +import java.sql.PreparedStatement;
    +import java.sql.ResultSet;
    +import java.sql.SQLException;
    +import java.util.Collection;
    +import java.util.concurrent.CountDownLatch;
    +import java.util.concurrent.TimeUnit;
    +
    +import org.apache.commons.logging.Log;
    +import org.apache.commons.logging.LogFactory;
    +import org.apache.phoenix.coprocessor.BaseScannerRegionObserver;
    +import org.apache.phoenix.metrics.Metrics;
    +import org.apache.phoenix.metrics.TracingTestCompat;
    +import org.apache.phoenix.trace.Hadoop1TracingTestEnabler.Hadoop1Disabled;
    +import org.apache.phoenix.trace.TraceReader.SpanInfo;
    +import org.apache.phoenix.trace.TraceReader.TraceHolder;
    +import org.cloudera.htrace.Sampler;
    +import org.cloudera.htrace.Span;
    +import org.cloudera.htrace.SpanReceiver;
    +import org.cloudera.htrace.Trace;
    +import org.cloudera.htrace.TraceScope;
    +import org.junit.After;
    +import org.junit.BeforeClass;
    +import org.junit.Test;
    +import org.junit.runner.RunWith;
    +
    +/**
    + * Test that the logging sink stores the expected metrics/stats
    + */
    +@RunWith(Hadoop1TracingTestEnabler.class)
    +@Hadoop1Disabled("tracing")
    +public class PhoenixTracingEndToEndIT extends BaseTracingTestIT {
    +
    +    private static final Log LOG = LogFactory.getLog(PhoenixTracingEndToEndIT.class);
    +    private static final int MAX_RETRIES = 10;
    +    private final String table = "ENABLED_FOR_LOGGING";
    +    private final String index = "ENABALED_FOR_LOGGING_INDEX";
    +
    +    private static DisableableMetricsWriter sink;
    +
    +    @BeforeClass
    +    public static void setupMetrics() throws Exception {
    +        if (shouldEarlyExitForHadoop1Test()) {
    +            return;
    +        }
    +        PhoenixTableMetricsWriter pWriter = new PhoenixTableMetricsWriter();
    +        Connection conn = getConnectionWithoutTracing();
    +        pWriter.initForTesting(conn);
    +        sink = new DisableableMetricsWriter(pWriter);
    +
    +        TracingTestCompat.registerSink(sink);
    +    }
    +
    +    @After
    +    public void cleanup() {
    +        sink.disable();
    +        sink.clear();
    +        sink.enable();
    +
    +        // LISTENABLE.clearListeners();
    +    }
    +
    +    private static void waitForCommit(CountDownLatch latch) throws SQLException {
    +        Connection conn = new CountDownConnection(getConnectionWithoutTracing(), latch);
    +        replaceWriterConnection(conn);
    +    }
    +
    +    private static void replaceWriterConnection(Connection conn) throws SQLException {
    +        // disable the writer
    +        sink.disable();
    +
    +        // swap the connection for one that listens
    +        sink.getDelegate().initForTesting(conn);
    +
    +        // enable the writer
    +        sink.enable();
    +    }
    +
    +    /**
    +     * Simple test that we can correctly write spans to the phoenix table
    +     * @throws Exception on failure
    +     */
    +    @Test
    +    public void testWriteSpans() throws Exception {
    +        // get a receiver for the spans
    +        SpanReceiver receiver = TracingCompat.newTraceMetricSource();
    +        // which also needs to a source for the metrics system
    +        Metrics.getManager().registerSource("testWriteSpans-source", "source for testWriteSpans",
    +            receiver);
    +
    +        // watch our sink so we know when commits happen
    +        CountDownLatch latch = new CountDownLatch(1);
    +        waitForCommit(latch);
    +
    +        // write some spans
    +        TraceScope trace = Trace.startSpan("Start write test", Sampler.ALWAYS);
    +        Span span = trace.getSpan();
    +
    +        // add a child with some annotations
    +        Span child = span.child("child 1");
    +        child.addTimelineAnnotation("timeline annotation");
    +        TracingCompat.addAnnotation(child, "test annotation", 10);
    +        child.stop();
    +
    +        // sleep a little bit to get some time difference
    +        Thread.sleep(100);
    +
    +        trace.close();
    +
    +        // pass the trace on
    +        receiver.receiveSpan(span);
    +
    +        // wait for the tracer to actually do the write
    +        latch.await();
    +
    +        // look for the writes to make sure they were made
    +        Connection conn = getConnectionWithoutTracing();
    +        checkStoredTraces(conn, new TraceChecker() {
    +            public boolean foundTrace(TraceHolder trace, SpanInfo info) {
    +                if (info.description.equals("child 1")) {
    +                    assertEquals("Not all annotations present", 1, info.annotationCount);
    +                    assertEquals("Not all tags present", 1, info.tagCount);
    +                    boolean found = false;
    +                    for (String annotation : info.annotations) {
    +                        if (annotation.startsWith("test annotation")) {
    +                            found = true;
    +                        }
    +                    }
    +                    assertTrue("Missing the annotations in span: " + info, found);
    +                    found = false;
    +                    for (String tag : info.tags) {
    +                        if (tag.endsWith("timeline annotation")) {
    +                            found = true;
    +                        }
    +                    }
    +                    assertTrue("Missing the tags in span: " + info, found);
    +                    return true;
    +                }
    +                return false;
    +            }
    +        });
    +    }
    +
    +    /**
    +     * Test that span will actually go into the this sink and be written on both side of the wire,
    +     * through the indexing code.
    +     * @throws Exception
    +     */
    +    @Test
    +    public void testClientServerIndexingTracing() throws Exception {
    +        // one call for client side, one call for server side
    +        final CountDownLatch updated = new CountDownLatch(2);
    +        waitForCommit(updated);
    +
    +        // separate connection so we don't create extra traces
    +        Connection conn = getConnectionWithoutTracing();
    +        createTestTable(conn, true);
    +
    +        // trace the requests we send
    +        Connection traceable = getTracingConnection();
    +        LOG.debug("Doing dummy the writes to the tracked table");
    +        String insert = "UPSERT INTO " + table + " VALUES (?, ?)";
    +        PreparedStatement stmt = traceable.prepareStatement(insert);
    +        stmt.setString(1, "key1");
    +        stmt.setLong(2, 1);
    +        // this first trace just does a simple open/close of the span. Its not doing anything
    +        // terribly interesting because we aren't auto-committing on the connection, so it just
    +        // updates the mutation state and returns.
    +        stmt.execute();
    +        stmt.setString(1, "key2");
    +        stmt.setLong(2, 2);
    +        stmt.execute();
    +        traceable.commit();
    +
    +        // wait for the latch to countdown, as the metrics system is time-based
    +        LOG.debug("Waiting for latch to complete!");
    +        updated.await(200, TimeUnit.SECONDS);// should be way more than GC pauses
    +
    +        // read the traces back out
    +
    +        /* Expected:
    +         * 1. Single element trace - for first PreparedStatement#execute span
    +         * 2. Two element trace for second PreparedStatement#execute span
    +         *  a. execute call
    +         *  b. metadata lookup*
    +         * 3. Commit trace.
    +         *  a. Committing to tables
    +         *    i. Committing to single table
    +         *    ii. hbase batch write*
    +         *    i.I. span on server
    +         *    i.II. building index updates
    +         *    i.III. waiting for latch
    +         * where '*' is a generically named thread (e.g phoenix-1-thread-X)
    --- End diff --
    
    Should/does this test querying the metrics table?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: Implement HTrace based tracing

Posted by jyates <gi...@git.apache.org>.
Github user jyates commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/5#discussion_r15407764
  
    --- Diff: phoenix-core/src/it/java/org/apache/phoenix/trace/PhoenixTracingEndToEndIT.java ---
    @@ -0,0 +1,401 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.phoenix.trace;
    +
    +import static org.junit.Assert.assertEquals;
    +import static org.junit.Assert.assertTrue;
    +
    +import java.sql.Connection;
    +import java.sql.PreparedStatement;
    +import java.sql.ResultSet;
    +import java.sql.SQLException;
    +import java.util.Collection;
    +import java.util.concurrent.CountDownLatch;
    +import java.util.concurrent.TimeUnit;
    +
    +import org.apache.commons.logging.Log;
    +import org.apache.commons.logging.LogFactory;
    +import org.apache.phoenix.coprocessor.BaseScannerRegionObserver;
    +import org.apache.phoenix.metrics.Metrics;
    +import org.apache.phoenix.metrics.TracingTestCompat;
    +import org.apache.phoenix.trace.Hadoop1TracingTestEnabler.Hadoop1Disabled;
    +import org.apache.phoenix.trace.TraceReader.SpanInfo;
    +import org.apache.phoenix.trace.TraceReader.TraceHolder;
    +import org.cloudera.htrace.Sampler;
    +import org.cloudera.htrace.Span;
    +import org.cloudera.htrace.SpanReceiver;
    +import org.cloudera.htrace.Trace;
    +import org.cloudera.htrace.TraceScope;
    +import org.junit.After;
    +import org.junit.BeforeClass;
    +import org.junit.Test;
    +import org.junit.runner.RunWith;
    +
    +/**
    + * Test that the logging sink stores the expected metrics/stats
    + */
    +@RunWith(Hadoop1TracingTestEnabler.class)
    +@Hadoop1Disabled("tracing")
    +public class PhoenixTracingEndToEndIT extends BaseTracingTestIT {
    +
    +    private static final Log LOG = LogFactory.getLog(PhoenixTracingEndToEndIT.class);
    +    private static final int MAX_RETRIES = 10;
    +    private final String table = "ENABLED_FOR_LOGGING";
    +    private final String index = "ENABALED_FOR_LOGGING_INDEX";
    +
    +    private static DisableableMetricsWriter sink;
    +
    +    @BeforeClass
    +    public static void setupMetrics() throws Exception {
    +        if (shouldEarlyExitForHadoop1Test()) {
    +            return;
    +        }
    +        PhoenixTableMetricsWriter pWriter = new PhoenixTableMetricsWriter();
    +        Connection conn = getConnectionWithoutTracing();
    +        pWriter.initForTesting(conn);
    +        sink = new DisableableMetricsWriter(pWriter);
    +
    +        TracingTestCompat.registerSink(sink);
    +    }
    +
    +    @After
    +    public void cleanup() {
    +        sink.disable();
    +        sink.clear();
    +        sink.enable();
    +
    +        // LISTENABLE.clearListeners();
    +    }
    +
    +    private static void waitForCommit(CountDownLatch latch) throws SQLException {
    +        Connection conn = new CountDownConnection(getConnectionWithoutTracing(), latch);
    +        replaceWriterConnection(conn);
    +    }
    +
    +    private static void replaceWriterConnection(Connection conn) throws SQLException {
    +        // disable the writer
    +        sink.disable();
    +
    +        // swap the connection for one that listens
    +        sink.getDelegate().initForTesting(conn);
    +
    +        // enable the writer
    +        sink.enable();
    +    }
    +
    +    /**
    +     * Simple test that we can correctly write spans to the phoenix table
    +     * @throws Exception on failure
    +     */
    +    @Test
    +    public void testWriteSpans() throws Exception {
    +        // get a receiver for the spans
    +        SpanReceiver receiver = TracingCompat.newTraceMetricSource();
    +        // which also needs to a source for the metrics system
    +        Metrics.getManager().registerSource("testWriteSpans-source", "source for testWriteSpans",
    +            receiver);
    +
    +        // watch our sink so we know when commits happen
    +        CountDownLatch latch = new CountDownLatch(1);
    +        waitForCommit(latch);
    +
    +        // write some spans
    +        TraceScope trace = Trace.startSpan("Start write test", Sampler.ALWAYS);
    +        Span span = trace.getSpan();
    +
    +        // add a child with some annotations
    +        Span child = span.child("child 1");
    +        child.addTimelineAnnotation("timeline annotation");
    +        TracingCompat.addAnnotation(child, "test annotation", 10);
    +        child.stop();
    +
    +        // sleep a little bit to get some time difference
    +        Thread.sleep(100);
    +
    +        trace.close();
    +
    +        // pass the trace on
    +        receiver.receiveSpan(span);
    +
    +        // wait for the tracer to actually do the write
    +        latch.await();
    +
    +        // look for the writes to make sure they were made
    +        Connection conn = getConnectionWithoutTracing();
    +        checkStoredTraces(conn, new TraceChecker() {
    +            public boolean foundTrace(TraceHolder trace, SpanInfo info) {
    +                if (info.description.equals("child 1")) {
    +                    assertEquals("Not all annotations present", 1, info.annotationCount);
    +                    assertEquals("Not all tags present", 1, info.tagCount);
    +                    boolean found = false;
    +                    for (String annotation : info.annotations) {
    +                        if (annotation.startsWith("test annotation")) {
    +                            found = true;
    +                        }
    +                    }
    +                    assertTrue("Missing the annotations in span: " + info, found);
    +                    found = false;
    +                    for (String tag : info.tags) {
    +                        if (tag.endsWith("timeline annotation")) {
    +                            found = true;
    +                        }
    +                    }
    +                    assertTrue("Missing the tags in span: " + info, found);
    +                    return true;
    +                }
    +                return false;
    +            }
    +        });
    +    }
    +
    +    /**
    +     * Test that span will actually go into the this sink and be written on both side of the wire,
    +     * through the indexing code.
    +     * @throws Exception
    +     */
    +    @Test
    +    public void testClientServerIndexingTracing() throws Exception {
    +        // one call for client side, one call for server side
    +        final CountDownLatch updated = new CountDownLatch(2);
    +        waitForCommit(updated);
    +
    +        // separate connection so we don't create extra traces
    +        Connection conn = getConnectionWithoutTracing();
    +        createTestTable(conn, true);
    +
    +        // trace the requests we send
    +        Connection traceable = getTracingConnection();
    +        LOG.debug("Doing dummy the writes to the tracked table");
    +        String insert = "UPSERT INTO " + table + " VALUES (?, ?)";
    +        PreparedStatement stmt = traceable.prepareStatement(insert);
    +        stmt.setString(1, "key1");
    +        stmt.setLong(2, 1);
    +        // this first trace just does a simple open/close of the span. Its not doing anything
    +        // terribly interesting because we aren't auto-committing on the connection, so it just
    +        // updates the mutation state and returns.
    +        stmt.execute();
    +        stmt.setString(1, "key2");
    +        stmt.setLong(2, 2);
    +        stmt.execute();
    +        traceable.commit();
    +
    +        // wait for the latch to countdown, as the metrics system is time-based
    +        LOG.debug("Waiting for latch to complete!");
    +        updated.await(200, TimeUnit.SECONDS);// should be way more than GC pauses
    +
    +        // read the traces back out
    +
    +        /* Expected:
    +         * 1. Single element trace - for first PreparedStatement#execute span
    +         * 2. Two element trace for second PreparedStatement#execute span
    +         *  a. execute call
    +         *  b. metadata lookup*
    +         * 3. Commit trace.
    +         *  a. Committing to tables
    +         *    i. Committing to single table
    +         *    ii. hbase batch write*
    +         *    i.I. span on server
    +         *    i.II. building index updates
    +         *    i.III. waiting for latch
    +         * where '*' is a generically named thread (e.g phoenix-1-thread-X)
    --- End diff --
    
    yes, it does, via the TraceReader


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: Implement HTrace based tracing

Posted by samarthjain <gi...@git.apache.org>.
Github user samarthjain commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/5#discussion_r15389352
  
    --- Diff: phoenix-core/src/it/java/org/apache/phoenix/trace/DisableableMetricsWriter.java ---
    @@ -0,0 +1,83 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.phoenix.trace;
    +
    +import java.sql.SQLException;
    +import java.util.concurrent.atomic.AtomicBoolean;
    +
    +import org.apache.commons.logging.Log;
    +import org.apache.commons.logging.LogFactory;
    +import org.apache.phoenix.metrics.MetricsWriter;
    +import org.apache.phoenix.metrics.PhoenixMetricsRecord;
    +
    +/**
    + *
    + */
    +public class DisableableMetricsWriter implements MetricsWriter {
    +
    +    private static final Log LOG = LogFactory.getLog(DisableableMetricsWriter.class);
    +    private PhoenixTableMetricsWriter writer;
    --- End diff --
    
    Looks like writer can be final. Also, is this class meant to be thread safe? If yes, maybe annotate with @ThreadSafe? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---